From ce466889ff758199978079adcbd30b1e7f05517e Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Fri, 22 May 2026 17:43:48 -0700 Subject: [PATCH] fix: remove line collapsing (#1330) Co-authored-by: open-swe[bot] --- agent/tools/add_finding.py | 26 +++++++++++++++----------- tests/test_reviewer_tools.py | 10 +++++----- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/agent/tools/add_finding.py b/agent/tools/add_finding.py index b657a5fe..64abd97b 100644 --- a/agent/tools/add_finding.py +++ b/agent/tools/add_finding.py @@ -55,14 +55,21 @@ def add_finding( ``style``, ``flag``, etc.). Free-form; used for grouping in the UI. file: Repo-relative path of the file the finding refers to. description: Markdown body the user sees. - start_line: 1-based line in the new (post-PR) file to anchor the - comment to. Pick the single most relevant line — the call site, - the signature, or the line the issue is actually about. Omit - (with ``end_line``) for file-level findings. - end_line: Accepted for API compatibility, but findings are always - anchored to a single line (``start_line``). GitHub renders - multi-line ranges as walls of context that bury the comment, so - we collapse to one line for a cleaner comment. + start_line: 1-based line in the new (post-PR) file where the + relevant range begins. For a single-line finding, this is the + line the issue is about. For a multi-line finding, this is the + first line of the relevant range. Omit (with ``end_line``) for + file-level findings. + end_line: 1-based line where the relevant range ends. GitHub + anchors the inline comment at ``end_line`` and renders the + ``start_line..end_line`` span as the highlighted snippet, so + choose ``end_line`` as the *last* line that matters — typically + the line the comment is most directly about. For a single-line + finding, set ``end_line == start_line`` (or omit it). Prefer + the natural range of the issue over a single line: GitHub + shows context above ``end_line``, so a one-line anchor often + buries the issue under unrelated context. Defaults to + ``start_line`` when omitted. suggestion: Replacement text for ``start_line..end_line``. When set, the published GitHub comment includes a ```suggestion``` block so the user can click "Commit suggestion". **Only set this for small, @@ -91,9 +98,6 @@ def add_finding( if start_line is not None and end_line is not None and end_line < start_line: return {"success": False, "error": "end_line must be >= start_line"} - if start_line is not None: - end_line = start_line - config = get_config() configurable = config.get("configurable", {}) if isinstance(config, dict) else {} diff_line_set = configurable.get("diff_line_set") if isinstance(configurable, dict) else None diff --git a/tests/test_reviewer_tools.py b/tests/test_reviewer_tools.py index 43fbce40..770b0dc1 100644 --- a/tests/test_reviewer_tools.py +++ b/tests/test_reviewer_tools.py @@ -98,7 +98,7 @@ def test_add_finding_persists_to_thread_metadata() -> None: assert persisted_thread == "tid-1" assert persisted["file"] == "foo.py" assert persisted["start_line"] == 11 - assert persisted["end_line"] == 11 + assert persisted["end_line"] == 12 assert persisted["suggestion"] == "renamed = 1" assert persisted["status"] == "open" assert persisted["first_seen_sha"] == "sha-head" @@ -197,8 +197,8 @@ def test_add_finding_keeps_short_suggestion() -> None: assert captured[0]["suggestion"] == short_suggestion -def test_add_finding_always_collapses_to_single_line() -> None: - """Multi-line ranges are always collapsed to ``start_line``.""" +def test_add_finding_preserves_multi_line_range() -> None: + """Multi-line ranges are preserved end-to-end (no collapse to start_line).""" captured: list[Any] = [] async def fake_append(thread_id: str, finding: Any) -> Any: @@ -215,14 +215,14 @@ def test_add_finding_always_collapses_to_single_line() -> None: confidence="low", category="style", file="foo.py", - description="anchor on start_line", + description="span the relevant range", start_line=15, end_line=19, ) assert result["success"] is True assert captured[0]["start_line"] == 15 - assert captured[0]["end_line"] == 15 + assert captured[0]["end_line"] == 19 def test_update_finding_rejects_long_suggestion_without_clobbering() -> None: