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

Dev minor #1536

Merged
merged 5 commits into from
Oct 31, 2024
Merged

Dev minor #1536

merged 5 commits into from
Oct 31, 2024

Conversation

emrgnt-cmplxty
Copy link
Contributor

@emrgnt-cmplxty emrgnt-cmplxty commented Oct 30, 2024

Important

This pull request enhances the R2R project's testing infrastructure, updates configurations for Azure, improves ingestion and orchestration processes, and refines prompt handling logic.

  • Testing Infrastructure:
    • Rename runner_sdk.py to runner_sdk_basic.py and update references in .github/actions/run-sdk-auth-tests/action.yml and other action files.
    • Add local_harness.py for running integration tests locally with detailed logging and result tracking.
    • Update test sequences in local_harness.py to include new test categories like sdk-prompts.
    • Add new test cases in test_prompt_handler.py for prompt caching and retrieval.
  • Configuration Changes:
    • Add full_azure.toml and r2r_azure.toml for Azure-specific configurations.
    • Update full.toml to use zerox PDF parser.
    • Modify pyproject.toml to change Python version requirement to >=3.10,<3.13.
  • Ingestion and Orchestration:
    • Update ingestion_router.py and kg_router.py to support run_with_orchestration flag for synchronous operations.
    • Modify R2RIngestionProvider and UnstructuredIngestionProvider to handle parser overrides and improve error handling.
  • Prompt Handling:
    • Enhance PostgresPromptHandler in prompt.py to improve cache invalidation and update logic.
    • Add tests for prompt update and retrieval in test_prompt_handler.py.

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

emrgnt-cmplxty and others added 2 commits October 30, 2024 11:37
* improve ci/cd runtime

* update prompt tests
Copy link

vercel bot commented Oct 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yc_demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 1:35am
yc-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 31, 2024 1:35am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
recommendation_platform ⬜️ Ignored (Inspect) Oct 31, 2024 1:35am

* improve ci/cd runtime

* update prompt tests

* add azure

* up

* up
* revert latest ingestion changes

* revert latest ingestion changes
@emrgnt-cmplxty emrgnt-cmplxty marked this pull request as ready for review October 31, 2024 01:39
@emrgnt-cmplxty emrgnt-cmplxty merged commit f1868c7 into main Oct 31, 2024
20 of 21 checks passed
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 9acc0e6 in 1 minute and 32 seconds

More details
  • Looked at 2581 lines of code in 35 files
  • Skipped 1 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. py/tests/integration/local_harness.py:126
  • Draft comment:
    The import statement has been updated to use runner_sdk_basic instead of runner_sdk. Ensure that this module exists and is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
2. py/tests/integration/runner_sdk_basic.py:56
  • Draft comment:
    The test function test_ingest_sample_file_3_sdk uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
3. py/tests/integration/runner_sdk_basic.py:103
  • Draft comment:
    The test function test_reingest_sample_file_sdk uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
4. py/tests/integration/runner_sdk_basic.py:204
  • Draft comment:
    The test function test_get_prompt uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
5. py/tests/integration/runner_sdk_basic.py:217
  • Draft comment:
    The test function test_rag_response_sample_file_sdk uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
6. py/tests/integration/runner_sdk_basic.py:233
  • Draft comment:
    The test function test_vector_search_sample_file_filter_sdk uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
7. py/tests/integration/runner_sdk_basic.py:248
  • Draft comment:
    The test function test_hybrid_search_sample_file_filter_sdk uses runner_sdk_basic.py instead of runner_sdk.py. Ensure that this module is correctly configured for the tests.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner script to use a new module runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a refactor or renaming of the test module. The change seems intentional and consistent.
8. py/core/configs/r2r_azure.toml:1
  • Draft comment:
    r2r_azure.toml is functionally similar to full_azure.toml except for orchestration settings. Consider extending full_azure.toml if orchestration is not needed.

  • full_azure.toml

  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment provides a suggestion to consider extending another configuration file (full_azure.toml) instead of creating a new one if orchestration is not needed. This could be a useful suggestion if it helps reduce redundancy or improve maintainability. However, without knowing the specific requirements or context of the project, it's speculative whether this suggestion is actionable or necessary.
    The comment assumes that the author might not need orchestration settings, which may not be the case. It also doesn't provide strong evidence that extending full_azure.toml is a better approach.
    The suggestion could be useful if the author is unaware of the similarities and the potential to extend full_azure.toml. However, without clear evidence that this change is needed, it remains speculative.
    The comment is speculative and does not provide strong evidence that a change is required. It should be deleted as it does not meet the criteria for a necessary code change.

Workflow ID: wflow_xW5ac0xrKCGhsWDq


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

@emrgnt-cmplxty emrgnt-cmplxty deleted the dev-minor branch November 4, 2024 18:27
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.

2 participants