feat: project governance llm checks #45
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to c18defa in 1 minute and 0 seconds
More details
- Looked at
119lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. src/datapilot/clients/altimate/client.py:99
- Draft comment:
Ensure exception handling is in place for thepostmethod call to handle potential request failures gracefully. - Reason this comment was not posted:
Comment did not seem useful.
2. src/datapilot/clients/altimate/client.py:95
- Draft comment:
Ensure exception handling is in place for thegetmethod call to handle potential request failures gracefully. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_BCGDTdFZfdaawF23
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 54ba983 in 11 seconds
More details
- Looked at
147lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/cli/cli.py:104
- Draft comment:
Unnecessary use oflento check ifllm_insightsis non-empty. Useif llm_insights:instead. - Reason this comment was not posted:
Confidence changes required:50%
The code inproject_healthfunction checks the length ofllm_insightsusinglen(llm_insights), which is unnecessary sinceif llm_insights:is sufficient to check if the list is non-empty.
Workflow ID: wflow_1JJEKRij4FdXXCJS
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 26380e3 in 28 seconds
More details
- Looked at
36lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:100
- Draft comment:
get_project_governance_llm_checksandrun_project_governance_llm_checksshould be imported fromdatapilot.clients.altimate.clientinstead ofdatapilot.clients.altimate.utils. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is not about a change made in the diff. The diff only shows changes to the arguments passed torun_project_governance_llm_checks, not where the functions are imported from. The comment does not provide strong evidence that the import source is incorrect or needs to be changed. It seems speculative without additional context.
I might be missing context about why the import source should be changed. The comment could be based on a broader understanding of the codebase that isn't visible in the diff.
Without evidence in the diff or a clear explanation, the comment does not meet the criteria for being kept. It seems speculative and unrelated to the changes made.
Delete the comment as it is not about a change made in the diff and lacks strong evidence for its suggestion.
Workflow ID: wflow_u1jlGKwNPFXpITaM
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on ae7423c in 21 seconds
More details
- Looked at
18lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/cli/cli.py:111
- Draft comment:
Ensure that the keys 'path', 'reason_to_flag', and 'recommendation' exist in the 'answer' dictionary. If the data structure has changed, this could lead to KeyError exceptions. - Reason this comment was not posted:
Confidence changes required:50%
The changes in the PR involve updating the keys used in theclick.echostatements. It's important to ensure that these keys match the structure of the data being processed.
Workflow ID: wflow_u2GP6tRWMXuJnzPC
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on bf82ad6 in 31 seconds
More details
- Looked at
102lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_4pupLVqcpB0S3lpl
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.
| metadata = answer.get("metadata", {}) | ||
| metadata["source"] = LLM | ||
| metadata["llm_id"] = report["id"] | ||
| metadata["catagory"] = report["type"] |
There was a problem hiding this comment.
Typo in metadata key: catagory should be category. This could lead to issues when accessing this metadata.
| metadata["catagory"] = report["type"] | |
| metadata["category"] = report["type"] |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 94d5563 in 23 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:190
- Draft comment:
Typo in variable name. Consider renamingcatagorytocategoryfor consistency.
metadata["category"] = report["type"]
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_GBwx0JJSpiUE9nBG
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 71dce56 in 11 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:189
- Draft comment:
Ensure that the change fromllm_idtoteammate_check_idis consistent across the codebase. Ifllm_idis used elsewhere, it should be updated toteammate_check_idfor consistency. - Reason this comment was not posted:
Confidence changes required:50%
The change fromllm_idtoteammate_check_idseems intentional and aligns with the context of the code, but it's important to ensure consistency across the codebase.
Workflow ID: wflow_B0y4GQZnmXKYPQE1
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on c636504 in 13 seconds
More details
- Looked at
40lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:111
- Draft comment:
Ensureself.manifestandself.cataloghave ajson()method before calling it, or handle the case where they might not have this method to avoid potential errors. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_CAxnuGLPP3l0LOnH
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 603be25 in 12 seconds
More details
- Looked at
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/cli/cli.py:30
- Draft comment:
Avoid setting default values in both the@click.optiondecorator and the function parameters. Set the default value in one place to prevent redundancy and potential maintenance issues. - Reason this comment was not posted:
Confidence changes required:50%
Theproject_healthfunction has a potential issue with the default value ofbackend_url. The default value is set both in the@click.optiondecorator and in the function parameter list. This redundancy can lead to maintenance issues and should be avoided.
Workflow ID: wflow_JmHsetzfiG63XeFs
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on de31c42 in 14 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:212
- Draft comment:
Useanswer.get("package_name", "")to simplify the conditional expression.
package_name=answer.get("package_name", ""),
- Reason this comment was not posted:
Confidence changes required:50%
The code uses a conditional expression to handle a missing key in a dictionary. This is a good practice to avoid KeyError, but it can be simplified using thedict.get()method.
Workflow ID: wflow_3iCWQ0ZZ51Wgee3Z
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 0cff308 in 31 seconds
More details
- Looked at
17lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_KBE6NZSCMcyX5IT8
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.
| severity=answer["severity"], | ||
| path=answer["path"] if answer.get("path") else "", | ||
| original_file_path=answer["original_file_path"] if answer.get("original_file_path") else "", | ||
| package_name=answer.get["package_name"] if answer.get("package_name") else "", |
There was a problem hiding this comment.
Incorrect syntax for accessing dictionary values. Use parentheses for the get method.
| package_name=answer.get["package_name"] if answer.get("package_name") else "", | |
| package_name=answer.get("package_name") if answer.get("package_name") else "", |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on dfd43ea in 20 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/datapilot/core/platforms/dbt/executor.py:191
- Draft comment:
Incorrect syntax for accessing dictionary values. Useanswer.get("package_name")instead ofanswer.get["package_name"]. This issue also appears on lines 210 and 211. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_e853KmPR3umDCvUL
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on a499fee in 28 seconds
More details
- Looked at
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_VFHxXlYl9MD35lcE
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.
| severity=answer["severity"], | ||
| path=answer["path"] if answer.get("path") else "", | ||
| original_file_path=answer["original_file_path"] if answer.get("original_file_path") else "", | ||
| package_name=answer["package_name"] if answer.get("package_name") else "", |
There was a problem hiding this comment.
Incorrect syntax for accessing dictionary values. Use answer.get("package_name") instead of answer.get["package_name"].
suryaiyer95
left a comment
suryaiyer95
left a comment
There was a problem hiding this comment.
Looks okay, @mdesmet Please have a look as well
|
@gaurpulkit Some tests are failing, Please have a look |
|
@gaurpulkit : The checks are failing, can you PTAL? |
mdesmet
left a comment
mdesmet
left a comment
There was a problem hiding this comment.
LGTM except 3.8 compatiblilty
|
Also is following scenario property handled: The user has a config that has LLM checks but forgets to provide the api key and tenant? |
Important
Adds project governance LLM checks to
APIClient, integrates them intoproject-healthCLI command, and updatesDBTInsightGeneratorto include these checks in reports.get_project_governance_llm_checks()andrun_project_governance_llm_checks()toAPIClientinclient.pyfor governance checks.project_health()incli.py, displaying results in CLI.--token,--instance-name, and--backend-urloptions toproject-healthcommand incli.py.project-healthcommand.get_project_governance_llm_checks()andrun_project_governance_llm_checks()inutils.pyfor API interaction.LLMconstant toconstants.pyfor categorizing LLM insights.DBTInsightGeneratorinexecutor.pyto run LLM checks and include results in reports.This description was created by
for a499fee. It will automatically update as commits are pushed.