Conversation
This adds a `create_review` GitLab implementation. It is not 1:1 to GitHub because you cannot approve + post review comments in the same request for GitLab.
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Line numbers incorrectly converted to strings for API
- Removed str() conversions so old_line and new_line are sent as JSON integers to match GitLab API expectations.
Or push these changes by commenting:
@cursor push ea0d25757f
Preview (ea0d25757f)
diff --git a/src/sentry/scm/private/providers/gitlab.py b/src/sentry/scm/private/providers/gitlab.py
--- a/src/sentry/scm/private/providers/gitlab.py
+++ b/src/sentry/scm/private/providers/gitlab.py
@@ -590,9 +590,9 @@
position["position_type"] = "text"
side = comment.get("side", "RIGHT")
if side == "LEFT":
- position["old_line"] = str(comment["line"])
+ position["old_line"] = comment["line"]
else:
- position["new_line"] = str(comment["line"])
+ position["new_line"] = comment["line"]
else:
position["position_type"] = "file"This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| if side == "LEFT": | ||
| position["old_line"] = str(comment["line"]) | ||
| else: | ||
| position["new_line"] = str(comment["line"]) |
There was a problem hiding this comment.
Line numbers incorrectly converted to strings for API
High Severity
The old_line and new_line values are wrapped in str(), converting them from integers to strings. Since the data dict is serialized as JSON (via json=data), this sends "old_line": "10" (a JSON string) instead of "old_line": 10 (a JSON number). The GitLab Discussions API documents position[new_line] and position[old_line] as integer parameters, so sending strings risks 400 errors or silent failures when creating inline review comments.
| if event == "change_request": | ||
| raise SCMProviderException("GitLab does not support REQUEST_CHANGES reviews") |
There was a problem hiding this comment.
Any thoughts on a better way to handle this with the new protocols?
There was a problem hiding this comment.
My and Colton's current approach is to split the method in two. We've done that for get_commits: we removed its path parameter and introduced get_commits_by_path.
jacquev6
left a comment
jacquev6
left a comment
There was a problem hiding this comment.
This deserves to be discussed more deeply in stand-up, so I'm not approving just yet.
| if event == "change_request": | ||
| raise SCMProviderException("GitLab does not support REQUEST_CHANGES reviews") |
There was a problem hiding this comment.
My and Colton's current approach is to split the method in two. We've done that for get_commits: we removed its path parameter and introduced get_commits_by_path.
| if comments: | ||
| versions = self.client.get_merge_request_versions(self._repo_id, pull_request_id) | ||
|
|
||
| for comment in comments: |
There was a problem hiding this comment.
We've not yet stated that in an explicit way, but we (Colton and I) have discussed that SCM methods should make a small, constant number of API call to the providers, i.e. be O(1).
We specifically discussed the create_review case about this question and were leaning towards "falling back to creating several comments is the client's responsibility".
(The fact that we need to log something when there are too many comments tends to strengthen the point that this is the client's responsibility.)
2 participants
This adds a
create_reviewGitLab implementation. It is not 1:1 to GitHub because you cannot approve + post review comments in the same request for GitLab and there is no equivalent to "changes requested".This GitLab implementation requires an API request per: