Fix Create Graph

[shreyaspimpalgaonkar]

Important

Enhance knowledge graph functionality by adding entity_table_name parameter to key functions and updating integration tests.

• Behavior:
  • Add entity_table_name parameter to get_entities() and get_entity_count() in kg.py, kg_service.py, and postgres.py.
  • Default entity_table_name to entity_embedding.
• Tests:
  • Update test_kg_create_graph_sample_file_cli() and test_kg_create_graph_sample_file_sdk() to check for at least one entity in the response.
  • Modify test_kg_enrich_graph_sample_file_cli() and test_kg_enrich_graph_sample_file_sdk() to validate community creation.
• Misc:
  • Fix minor issues in runner_cli.py and runner_sdk.py related to entity and community assertions.

^{This description was created by for 181aeee. 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 11, 2024 5:00pm
yc-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 11, 2024 5:00pm

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
recommendation_platform	⬜️ Ignored (Inspect)			Oct 11, 2024 5:00pm

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 5b7397c in 49 seconds

More details

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

1. py/tests/integration/runner_cli.py:259

• Draft comment:
  Consider checking for specific expected entities in the entities_list to make the test more robust, rather than just checking if the list has at least one entity.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The test function test_kg_create_graph_sample_file_cli in runner_cli.py and test_kg_create_graph_sample_file_sdk in runner_sdk.py both check for the presence of entities after creating a graph. However, the assertion only checks if the length of the entities list is greater than or equal to 1, which is a very minimal check. It would be more robust to check for specific expected entities if possible.

2. py/tests/integration/runner_cli.py:280

• Draft comment:
  Consider checking for specific expected community attributes in the communities list to make the test more robust, rather than just checking if the list has at least one community.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The test function test_kg_enrich_graph_sample_file_cli in runner_cli.py and test_kg_enrich_graph_sample_file_sdk in runner_sdk.py both check for the presence of communities after enriching a graph. However, the assertion only checks if the length of the communities list is greater than or equal to 1, which is a very minimal check. It would be more robust to check for specific expected community attributes if possible.

Workflow ID: wflow_4UUMPh1WvJ8bc9aS

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on d156f81 in 36 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/runner_cli.py:261

• Draft comment:
  The assertion assert "ARISTOTLE" in entities_list is redundant if the test's purpose is only to check for at least one entity. Consider removing it unless specifically required.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The assertion for checking if 'ARISTOTLE' is in the entities list is redundant because the test already checks for at least one entity. This specific check should be removed unless there's a specific reason to check for 'ARISTOTLE'.

2. py/tests/integration/runner_sdk.py:670

• Draft comment:
  The assertion assert "ARISTOTLE" in entities_list is redundant if the test's purpose is only to check for at least one entity. Consider removing it unless specifically required.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The assertion for checking if 'ARISTOTLE' is in the entities list is redundant because the test already checks for at least one entity. This specific check should be removed unless there's a specific reason to check for 'ARISTOTLE'.

Workflow ID: wflow_divtpHMGAyE9SsG7

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on e1999d9 in 33 seconds

More details

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

1. py/tests/integration/runner_cli.py:259

• Draft comment:
  Remove the print statement used for debugging purposes to maintain clean test output.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The print statement added for debugging purposes should be removed to maintain clean test output.

2. py/tests/integration/runner_sdk.py:668

• Draft comment:
  Remove the print statement used for debugging purposes to maintain clean test output.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The print statement added for debugging purposes should be removed to maintain clean test output.

Workflow ID: wflow_2Ia4SV3m4cOeNOJs

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 651014d in 49 seconds

More details

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

1. py/tests/integration/runner_cli.py:246

• Draft comment:
  The entity_table_name parameter is missing in the request. Consider adding it to align with the changes mentioned in the PR description.
• Reason this comment was not posted:
  Comment did not seem useful.

2. py/tests/integration/runner_sdk.py:662

• Draft comment:
  The entity_table_name parameter is missing in the get_entities call. Consider adding it to align with the changes mentioned in the PR description.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment seems speculative as it references the PR description, which is not available. The code change only involves adding a limit parameter, and there is no indication that entity_table_name is required. Without strong evidence, the comment should be removed.
  I might be missing context from the PR description that could justify the comment. However, without that context, the comment appears speculative.
  The rules state to remove comments unless there is strong evidence they are correct. Without the PR description, the comment lacks strong evidence.
  Remove the comment as it is speculative and lacks strong evidence of correctness.

Workflow ID: wflow_QWNEjuKaERmo88T2

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 1211e2f in 28 seconds

More details

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

1. .github/actions/run-cli-graphrag-tests/action.yml:9

• Draft comment:
  The function test_ingest_sample_file_cli is still present in runner_cli.py but is not used in the action files. Consider removing it if test_ingest_sample_file_2_cli is intended to replace it.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The change in the action.yml files from test_ingest_sample_file_cli to test_ingest_sample_file_2_cli is consistent with the addition of the new function test_ingest_sample_file_2_cli in runner_cli.py. However, the original function test_ingest_sample_file_cli is still present and unused, which might be unnecessary if the new function is intended to replace it.

Workflow ID: wflow_GhjgaPlfHeKAsdqb

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 9687283 in 34 seconds

More details

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

1. py/tests/integration/runner_sdk.py:662

• Draft comment:
  The get_entities function call is missing the entity_table_name parameter, which was mentioned in the PR description. Ensure this parameter is included if required.
• Reason this comment was not posted:
  Comment did not seem useful.

2. py/tests/integration/runner_cli.py:42

• Draft comment:
  Remove the unnecessary newline to maintain code style consistency.
• Reason this comment was not posted:
  Confidence changes required: 10%
  There is an extra newline added which is unnecessary and doesn't follow the existing code style.

Workflow ID: wflow_AfEx3PniuUVYO1n5

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 58e83d2 in 38 seconds

More details

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

1. .github/actions/run-cli-graphrag-tests/action.yml:1

• Draft comment:
  The step name 'Ingest sample file (SDK)' seems incorrect for a CLI test. It should be 'Ingest sample file (CLI)'.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_xFhkbyHfNMAp8gDC

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 181aeee in 40 seconds

More details

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

1. py/tests/integration/runner_sdk.py:36

• Draft comment:
  The function test_ingest_sample_file_2_sdk is redundant as it duplicates the logic of test_ingest_sample_file_sdk. Consider removing it to avoid unnecessary duplication.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment points out a potential redundancy, but the functions use different file paths, which could be intentional for testing different files. Without more context on the purpose of these tests, it's hard to say if the redundancy is an issue. The comment might be speculative without strong evidence of an issue.
  The comment assumes redundancy without considering the possibility that different file paths are used intentionally for different test cases. It lacks strong evidence that the redundancy is problematic.
  The use of different file paths suggests that the functions might be testing different scenarios, which is a valid reason to keep both functions. The comment does not provide strong evidence that the redundancy is an issue.
  The comment should be deleted because it lacks strong evidence that the redundancy is problematic, given the different file paths used in the functions.

Workflow ID: wflow_CnQTZN5fOyPBrMRp

---

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