fix: handle GitLab single-line diff notes in /ask_line

Author: Evans Castonguay

pr_agent/servers/gitlab_webhook.py

@@ -277,17 +277,33 @@ async def inner(data: dict):
 
 def handle_ask_line(body, data):
     try:
-        line_range_ = data['object_attributes']['position']['line_range']
-        # if line_range_['start']['type'] == 'new':
-        start_line = line_range_['start']['new_line']
-        end_line = line_range_['end']['new_line']
-        # else:
-        #     start_line = line_range_['start']['old_line']
-        #     end_line = line_range_['end']['old_line']
+        position = data.get('object_attributes', {}).get('position') or {}
+        line_range_ = position.get('line_range')
+
+        if line_range_:
+            range_type = line_range_.get('start', {}).get('type')
+            side = 'RIGHT' if range_type == 'new' else 'LEFT'
+            if range_type == 'new':
+                start_line = line_range_.get('start', {}).get('new_line')
+                end_line = line_range_.get('end', {}).get('new_line')
+                path = position.get('new_path')
+            else:
+                start_line = line_range_.get('start', {}).get('old_line')
+                end_line = line_range_.get('end', {}).get('old_line')
+                path = position.get('old_path')
+        else:
+            start_line = position.get('new_line') or position.get('old_line')
+            end_line = position.get('new_line') or position.get('old_line')
+            side = 'RIGHT' if position.get('new_line') is not None else 'LEFT'
+            path = position.get('new_path') or position.get('old_path')
+
+        if start_line is None or end_line is None:
+            raise ValueError("Missing line numbers in diff position")
+        if not path:
+            raise ValueError("Missing file path in diff position")
+
         question = body.replace('/ask', '').strip()
-        path = data['object_attributes']['position']['new_path']
-        side = 'RIGHT'  # if line_range_['start']['type'] == 'new' else 'LEFT'
-        comment_id = data['object_attributes']["discussion_id"]
+        comment_id = data.get('object_attributes', {}).get("discussion_id", "")
         get_logger().info("Handling line ")
         body = f"/ask_line --line_start={start_line} --line_end={end_line} --side={side} --file_name={path} --comment_id={comment_id} {question}"
     except Exception as e:

tests/unittest/test_gitlab_webhook.py

@@ -0,0 +1,71 @@
+import importlib
+import os
+
+import pytest
+
+
+@pytest.fixture(scope="module", autouse=True)
+def _set_gitlab_url():
+    os.environ.setdefault("GITLAB__URL", "https://gitlab.example.com")
+    yield
+
+
+def _load_module():
+    import pr_agent.servers.gitlab_webhook as gitlab_webhook
+
+    return importlib.reload(gitlab_webhook)
+
+
+def _base_payload(position):
+    return {
+        "object_attributes": {
+            "position": position,
+            "discussion_id": "discussion-1",
+        }
+    }
+
+
+@pytest.mark.parametrize(
+    "line_range_type,expected_side,start_key,end_key,path_key",
+    [
+        ("new", "RIGHT", "new_line", "new_line", "new_path"),
+        ("old", "LEFT", "old_line", "old_line", "old_path"),
+    ],
+)
+def test_handle_ask_line_line_range(line_range_type, expected_side, start_key, end_key, path_key):
+    module = _load_module()
+    position = {
+        "line_range": {
+            "start": {start_key: 10, "type": line_range_type},
+            "end": {end_key: 12, "type": line_range_type},
+        },
+        "new_path": "src/new.py",
+        "old_path": "src/old.py",
+    }
+    payload = _base_payload(position)
+
+    result = module.handle_ask_line("/ask what is this?", payload)
+
+    assert f"--line_start=10" in result
+    assert f"--line_end=12" in result
+    assert f"--side={expected_side}" in result
+    assert f"--file_name={position[path_key]}" in result
+
+
+@pytest.mark.parametrize(
+    "position,expected_side,expected_path",
+    [
+        ({"new_line": 5, "new_path": "src/new.py"}, "RIGHT", "src/new.py"),
+        ({"old_line": 7, "old_path": "src/old.py"}, "LEFT", "src/old.py"),
+    ],
+)
+def test_handle_ask_line_single_line(position, expected_side, expected_path):
+    module = _load_module()
+    payload = _base_payload(position)
+
+    result = module.handle_ask_line("/ask explain", payload)
+
+    assert "--line_start" in result
+    assert "--line_end" in result
+    assert f"--side={expected_side}" in result
+    assert f"--file_name={expected_path}" in result