adam/open-swe
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/open-swe.git synced 2026-10-06 11:12:10 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

fix: remove line collapsing (#1330)

Browse source 
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
Johannes du Plessis 2026-05-22 17:43:48 -07:00 • committed by GitHub
parent 72252e48c9
commit ce466889ff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 20 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
agent/tools/add_finding.py
View file

@ -55,14 +55,21 @@ def add_finding(
``style``, ``flag``, etc.). Free-form; used for grouping in the UI. ``style``, ``flag``, etc.). Free-form; used for grouping in the UI.
file: Repo-relative path of the file the finding refers to. file: Repo-relative path of the file the finding refers to.
description: Markdown body the user sees. description: Markdown body the user sees.
start_line: 1-based line in the new (post-PR) file to anchor the start_line: 1-based line in the new (post-PR) file where the
comment to. Pick the single most relevant line — the call site, relevant range begins. For a single-line finding, this is the
the signature, or the line the issue is actually about. Omit line the issue is about. For a multi-line finding, this is the
(with ``end_line``) for file-level findings. first line of the relevant range. Omit (with ``end_line``) for
end_line: Accepted for API compatibility, but findings are always file-level findings.
anchored to a single line (``start_line``). GitHub renders end_line: 1-based line where the relevant range ends. GitHub
multi-line ranges as walls of context that bury the comment, so anchors the inline comment at ``end_line`` and renders the
we collapse to one line for a cleaner comment. ``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, suggestion: Replacement text for ``start_line..end_line``. When set,
the published GitHub comment includes a ```suggestion``` block so the published GitHub comment includes a ```suggestion``` block so
the user can click "Commit suggestion". **Only set this for small, 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: 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"} return {"success": False, "error": "end_line must be >= start_line"}
if start_line is not None:
end_line = start_line
config = get_config() config = get_config()
configurable = config.get("configurable", {}) if isinstance(config, dict) else {} configurable = config.get("configurable", {}) if isinstance(config, dict) else {}
diff_line_set = configurable.get("diff_line_set") if isinstance(configurable, dict) else None diff_line_set = configurable.get("diff_line_set") if isinstance(configurable, dict) else None

10
tests/test_reviewer_tools.py
View file

@ -98,7 +98,7 @@ def test_add_finding_persists_to_thread_metadata() -> None:
assert persisted_thread == "tid-1" assert persisted_thread == "tid-1"
assert persisted["file"] == "foo.py" assert persisted["file"] == "foo.py"
assert persisted["start_line"] == 11 assert persisted["start_line"] == 11
assert persisted["end_line"] == 11 assert persisted["end_line"] == 12
assert persisted["suggestion"] == "renamed = 1" assert persisted["suggestion"] == "renamed = 1"
assert persisted["status"] == "open" assert persisted["status"] == "open"
assert persisted["first_seen_sha"] == "sha-head" 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 assert captured[0]["suggestion"] == short_suggestion
def test_add_finding_always_collapses_to_single_line() -> None: def test_add_finding_preserves_multi_line_range() -> None:
"""Multi-line ranges are always collapsed to ``start_line``.""" """Multi-line ranges are preserved end-to-end (no collapse to start_line)."""
captured: list[Any] = [] captured: list[Any] = []
async def fake_append(thread_id: str, finding: Any) -> 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", confidence="low",
category="style", category="style",
file="foo.py", file="foo.py",
description="anchor on start_line", description="span the relevant range",
start_line=15, start_line=15,
end_line=19, end_line=19,
) )
assert result["success"] is True assert result["success"] is True
assert captured[0]["start_line"] == 15 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: def test_update_finding_rejects_long_suggestion_without_clobbering() -> None: