fix(gitlab): preserve context lines in suggestions

[noooooooookro]

What changed

• Derive GitLab suggestion positions from hunk context instead of always marking them as additions.
• Send both old_line and new_line for context anchors while preserving the general-note fallback outside the diff.
• Add regression coverage for context-line suggestions.

Why

GitLab requires both line numbers for a context-line position. Previously, /improve suggestions anchored to context lines were rejected and degraded to a general note.

Validation

• PYTHONPATH=. python -m pytest tests/unittest/test_gitlab_provider.py tests/unittest/test_gitlab_batch_publish_suggestions.py tests/unittest/test_gitlab_publish_guard.py -q (180 passed)
• Ruff check on the two changed files (passed)

Closes #3131

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)

1. Patch parsing is tied to publishing 📘 Rule violation ⚙ Maintainability

Description

publish_code_suggestions now parses hunk headers and tracks old and new line counters inline while
also coordinating GitLab note publication. Any future change to patch-position semantics must
therefore be made inside the publishing workflow and exercised through its provider setup, coupling
data transformation to external-note orchestration.

Code

pr_agent/git_providers/gitlab_provider.py[R1289-1292]

+                for patch_line in (target_file.patch or '').splitlines():
+                    if patch_line.startswith('@@'):
+                        match = self.RE_HUNK_HEADER.match(patch_line)
+                        if match:

Evidence

Compliance rule 2694714 separates data transformation from workflow coordination. The added lines
parse and transform patch-position data inside a function that subsequently calls
send_inline_comment to perform the publishing workflow.

Rule 2694714: Limit functions to a single, clearly defined responsibility
pr_agent/git_providers/gitlab_provider.py[1287-1308]
pr_agent/git_providers/gitlab_provider.py[1319-1321]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`publish_code_suggestions` mixes positional hunk parsing with the workflow that publishes GitLab suggestions, giving the function separate transformation and orchestration responsibilities.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[1287-1308]
## Recommended Fix
Extract the hunk traversal and old/new line calculation into a focused helper that accepts the patch and requested line and returns the edit type and resolved line numbers. Call that helper from `publish_code_suggestions`, retaining the existing fallback when it reports no match.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Two anchor comments use narrative prose 📘 Rule violation ⚙ Maintainability

Description

The new comment blocks use declarative clauses—A content search stops... and `GitLab will
reject...`—instead of phrasing those behavioral notes imperatively. Future readers encounter both
statements beside the hunk scan and fallback branch and must infer whether they are instructions to
preserve or merely historical observations.

Code

pr_agent/git_providers/gitlab_provider.py[R1284-1286]

+                # Classify the anchor positionally from the hunk headers. A content search stops
+                # at the first line holding the same text, which moves the anchor when that text
+                # repeats earlier in the patch, and the body is a -0+N window that travels with it.

Evidence

Compliance rule 2694688 requires behavioral comments to use imperative phrasing. The cited additions
contain two declarative behavioral explanations in newly added comment blocks.

Rule 2694688: Docstrings and comments must use imperative phrasing
pr_agent/git_providers/gitlab_provider.py[1284-1286]
pr_agent/git_providers/gitlab_provider.py[1311-1313]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Two newly added anchor-handling comments describe behavior with declarative narrative clauses rather than imperative phrasing.
## Fix Focus Areas
- pr_agent/git_providers/gitlab_provider.py[1284-1286]
- pr_agent/git_providers/gitlab_provider.py[1311-1313]
## Recommended Fix
Rewrite the first block with an instruction such as `Avoid a content search because...`, and rewrite the fallback explanation as an imperative instruction while preserving its technical meaning.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[IsmaelMartinez]

Thanks, and the reported case is fixed: both line numbers now go out for a context-line anchor. One blocker and one point about the fake, both inline with suggestions.

[IsmaelMartinez]

Suggested change

	if relevant_line_in_file:
	edit_type, found, source_line_no, target_file, target_line_no = self.find_in_file(
	target_file, relevant_line_in_file
	)
	else:
	found = False
	if relevant_line_in_file:
	# Classify the anchor positionally from the hunk headers. A content search
	# stops at the first line holding the same text, which moves the anchor when
	# that text repeats earlier in the patch, and the body is a -0+N window that
	# travels with it.
	edit_type, found, source_line_no, target_line_no = 'addition', False, -1, 0
	old_line_no = new_line_no = 0
	for patch_line in (target_file.patch or '').splitlines():
	if patch_line.startswith('@@'):
	match = self.RE_HUNK_HEADER.match(patch_line)
	if match:
	old_line_no, new_line_no = int(match.group(1)), int(match.group(3))
	continue
	if patch_line.startswith('\\'):
	continue
	if patch_line.startswith('-'):
	old_line_no += 1
	continue
	if patch_line.startswith('+'):
	new_line_no += 1
	else:
	old_line_no += 1
	new_line_no += 1
	if new_line_no - 1 == relevant_lines_start:
	edit_type = 'addition' if patch_line.startswith('+') else 'context'
	found, source_line_no, target_line_no = True, old_line_no, new_line_no
	break
	else:
	found = False

find_in_file is a substring scan that stops at the first line containing the text and ignores relevant_lines_start, so any anchor whose text repeats earlier in the patch relocates. Measured against main:

• added line 8 whose text also appears as context line 2: main sends new_line: 8, this sends new_line: 2
• added line 3 whose text also appears as a deleted line: main sends new_line: 3, this sends old_line: 1 with no new_line at all, so the suggestion anchors on the old file

Both are right on main today. The body is still suggestion:-0+N, computed from relevant_lines_end minus relevant_lines_start before this runs, so the replacement window travels with the anchor and Apply overwrites the wrong lines.

The suggestion is the positional shape from #3131: walk the hunk headers of target_file.patch, classify the line at relevant_lines_start from its prefix, and leave the anchor where the model put it. Your if not found: fallback below stays exactly as written. I ran it: all three cases above come out right, your own test file still passes at 9, the full suite is unchanged at 4338 and ruff is clean.

[IsmaelMartinez]

Suggested change

	patch = "@@ -1,3 +1,3 @@\n line1\n line2\n line3\n"
	patch = "@@ -1,2 +1,3 @@\n line1\n line2\n+line3\n"

An all-context hunk is something git never emits, so as written the fake cannot exercise the added-line path at all. This makes +line3 a real addition, consistent with head_file, and all nine tests stay green under it either way; I checked both.

It still will not catch a relocated anchor, because every line in the fake is unique. For that, head_file needs a line whose text repeats, a fourth line reading line2 say, with a row anchoring relevant_lines_start on it. I could not fold that into the suggestion because head_file is not a changed line in this diff.

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 64ea566

[IsmaelMartinez]

Thanks for the fix and the test file; sorry this sat for eight days after the review.

I have pushed the two 8 September suggestions to your branch as 64ea566 rather than wait longer, since #3131 was picked up elsewhere today: the positional hunk walk (without the blank-line guard, so an empty context line also gets both numbers) and the real-hunk fake, plus a test that anchors on a repeated and on a blank context line. All eight anchor shapes I tried come out right, and the suite is green on today's main.

Merging once CI is green so #3131 can close; the fix stays yours.

[IsmaelMartinez]

Carried the 8 September suggestions myself (64ea566); eight anchor shapes verified, suite green on today's main.