Add KG tests

[shreyaspimpalgaonkar]

Important

Add CLI and SDK tests for Knowledge Graph operations and update workflow to include these tests.

• Tests:
  • Add CLI tests for KG operations in harness_cli.py: test_kg_create_graph_sample_file_cli, test_kg_enrich_graph_sample_file_cli, test_kg_search_sample_file_cli.
  • Add SDK tests for KG operations in harness_sdk.py: test_kg_create_graph_sample_file_sdk, test_kg_enrich_graph_sample_file_sdk, test_kg_search_sample_file_sdk.
• Workflow:
  • Update integration-test-workflow-debian.yml to include new KG tests for both CLI and SDK.

^{This description was created by for 6617050. It will automatically update as commits are pushed.}

[vercel]

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 8, 2024 2:07am
yc-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 8, 2024 2:07am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
recommendation_platform	⬜️ Ignored (Inspect)			Oct 8, 2024 2:07am

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 337436c in 28 seconds

More details

• Looked at 352 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. py/core/main/api/kg_router.py:261

• Draft comment:
  Permission checks for non-superusers are not implemented. This is a security risk as it could allow unauthorized access. Similar issues are present at lines 201, 230, and 262.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment highlights a legitimate security concern in the newly added 'get_communities' function. The code has a warning to implement permission checks, which aligns with the comment's observation. The mention of similar issues at other lines is not relevant to this diff, but the issue in the new code is valid.
  The comment mentions other lines that are not part of the new changes, which could be misleading. However, the core issue it raises about the new function is valid.
  While the comment could be more focused on the new changes, the security risk it highlights in the new code is important and actionable.
  Keep the comment as it correctly identifies a security risk in the new code that needs to be addressed.

2. py/core/providers/kg/postgres.py:516

• Draft comment:
  Consider adding a return type annotation for clarity and consistency. This applies to other methods without return type annotations as well.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The get_communities method in postgres.py is missing a return type annotation. This is a minor issue but should be corrected for consistency and clarity.

3. py/tests/integration/harness_cli.py:243

• Draft comment:
  Avoid using hardcoded collection IDs. Consider parameterizing the collection ID to make the test more flexible and robust.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In harness_cli.py, the test_kg_create_graph_sample_file_cli function uses a hardcoded collection ID. This could lead to issues if the collection ID changes or is different in other environments.

4. py/tests/integration/harness_sdk.py:603

• Draft comment:
  Avoid using hardcoded collection IDs. Consider parameterizing the collection ID to make the test more flexible and robust.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In harness_sdk.py, the test_kg_create_graph_sample_file_sdk function uses a hardcoded collection ID. This could lead to issues if the collection ID changes or is different in other environments.

Workflow ID: wflow_t5xhA6ZoZra9pGDo

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 86e8145 in 17 seconds

More details

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

1. .github/workflows/integration-test-workflow-debian.yml:38

• Draft comment:
  Move the export PATH="/root/.local/bin:$PATH" command to the same step where Poetry is installed to ensure it is available for subsequent commands.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment is suggesting a change that is already implemented in the code. The 'export PATH' command is indeed in the same step as the Poetry installation, so the comment is redundant. There is no code change required based on this comment.
  I might be missing some context about why the comment was made, but based on the current code, the issue seems resolved. The comment might have been relevant to a previous version of the code.
  The current code structure already addresses the concern raised by the comment, so it is not necessary to keep it.
  The comment is redundant as the suggested change is already implemented. It should be deleted.

Workflow ID: wflow_GqeZSt4pHFKO1MfM

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 5a48604 in 19 seconds

More details

• Looked at 27 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. py/tests/integration/harness_cli.py:285

• Draft comment:
  The change from assert len(communities) >= 10 to assert len(communities) >= 1 should be documented or justified in the PR description to ensure clarity on why the expected number of communities has decreased.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The change in the assertion from 10 to 1 in both CLI and SDK tests for KG enrichment is significant. It might be due to a change in the expected output or a bug fix. However, the change should be documented or justified in the PR description to ensure clarity.

2. py/tests/integration/harness_sdk.py:663

• Draft comment:
  The change from assert len(communities) >= 10 to assert len(communities) >= 1 should be documented or justified in the PR description to ensure clarity on why the expected number of communities has decreased.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The change in the assertion from 10 to 1 in both CLI and SDK tests for KG enrichment is significant. It might be due to a change in the expected output or a bug fix. However, the change should be documented or justified in the PR description to ensure clarity.

Workflow ID: wflow_XLEnUKCi8tHU3Uy6

---

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

[ellipsis-dev]

❌ Changes requested. Incremental review on 6617050 in 23 seconds

More details

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

Workflow ID: wflow_XQltZKnMjUNv36K8

---

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.

[ellipsis-dev]

The test test_remove_all_files_and_ingest_sample_file_sdk is incorrectly repeated in the CLI GraphRAG section. It should be replaced with the correct CLI test, likely test_remove_all_files_and_ingest_sample_file_cli.