-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: Add ability to fetch step from data layer #2729
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
base: main
Are you sure you want to change the base?
feat: Add ability to fetch step from data layer #2729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/chainlit/data/sql_alchemy.py">
<violation number="1" location="backend/chainlit/data/sql_alchemy.py:451">
P1: Missing empty list check before accessing `steps_feedbacks[0]`. If the step doesn't exist, the query returns an empty list and this will raise an `IndexError`. Follow the existing pattern used elsewhere in this file (e.g., `get_user`, `get_element`).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
backend/chainlit/data/sql_alchemy.py
Outdated
| query=steps_feedbacks_query, parameters={"step_id": step_id} | ||
| ) | ||
|
|
||
| if not isinstance(steps_feedbacks, list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Missing empty list check before accessing steps_feedbacks[0]. If the step doesn't exist, the query returns an empty list and this will raise an IndexError. Follow the existing pattern used elsewhere in this file (e.g., get_user, get_element).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/chainlit/data/sql_alchemy.py, line 451:
<comment>Missing empty list check before accessing `steps_feedbacks[0]`. If the step doesn't exist, the query returns an empty list and this will raise an `IndexError`. Follow the existing pattern used elsewhere in this file (e.g., `get_user`, `get_element`).</comment>
<file context>
@@ -415,6 +415,83 @@ async def delete_step(self, step_id: str):
+ query=steps_feedbacks_query, parameters={"step_id": step_id}
+ )
+
+ if not isinstance(steps_feedbacks, list):
+ return None
+
</file context>
| if not isinstance(steps_feedbacks, list): | |
| if not isinstance(steps_feedbacks, list) or not steps_feedbacks: |
✅ Addressed in abbdfa9
090982f to
abbdfa9
Compare
This is useful for pulling message metadata from an on_feedback callback, for instance.
Head branch was pushed to by a user without write access
abbdfa9 to
32f5d82
Compare
|
v2:
|
This is useful for pulling message metadata from an on_feedback callback, for instance.
Summary by cubic
Add get_step to the data layer to fetch a step by ID with its associated feedback. This enables pulling message metadata in on_feedback callbacks.
Written for commit 32f5d82. Summary will update automatically on new commits.