mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-02 23:53:26 +00:00
Adam Moussa
62d9945df4
Part of the domain-reorg adoption (build plan step C2): fork content,
upstream layout. Nine 1:1 module moves (reviewer_diff/eval_store/
findings/groups/publish/reconcile/trace_context + review_style_
collector/guidance) into agent/review/, with internal relative
imports re-wired to the new package depth. agent/review/__init__.py
mirrors upstream's thin re-export shim (one of the 21 verified "A"
structural adds).
Rewrote the 38 grep hits across importer files (agent/{analyzer,
ci_autofix,reviewer,webapp}.py, agent/dashboard/*, agent/middleware/
settle_review_check.py, agent/tools/*, agent/utils/github_feedback.py,
agent/webhooks/github.py, evals/reviewer/*, and the reviewer test
suite) to point at agent.review.*; 4 of the 38 hits were name
collisions (list_reviewer_findings, reviewer_outcomes,
_reviewer_thread_id, reviewer_thread_id — not the moved modules) and
were left untouched. tests/test_github_checks.py's module-alias
import (`from agent import reviewer_publish`) follows upstream's own
`from agent.review import publish as reviewer_publish` pattern so
downstream `reviewer_publish.*` call sites needed no changes.
agent/reviewer.py and agent/webapp.py stay in place per the hard
rule (fork content, import-only rewire) and are not part of this
package.
Gates: ruff check + ruff format --check, pytest --co -q (1637
collected), full unit suite (1637 passed), and the reviewer/findings
suite in isolation (pytest -k "review or finding", 421 passed).
297 lines
9.6 KiB
Python
297 lines
9.6 KiB
Python
from __future__ import annotations
|
|
|
|
from unittest.mock import AsyncMock, patch
|
|
|
|
import pytest
|
|
|
|
from agent.review.reconcile import reconcile_findings_with_review_threads
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_marks_resolved_github_thread_resolved() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": 11,
|
|
"github_review_thread_id": "THREAD_1",
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_1",
|
|
"is_resolved": True,
|
|
"is_outdated": False,
|
|
"comments": [{"id": 11, "author": "open-swe[bot]", "body": "bug"}],
|
|
}
|
|
],
|
|
)
|
|
|
|
assert result[0]["status"] == "resolved"
|
|
assert result[0]["github_thread_resolved"] is True
|
|
replace.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_backfills_comment_and_thread_ids_from_bot_marker() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": None,
|
|
"github_review_thread_id": None,
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_1",
|
|
"is_resolved": False,
|
|
"is_outdated": False,
|
|
"comments": [
|
|
{
|
|
"id": 11,
|
|
"author": "open-swe[bot]",
|
|
"body": (
|
|
'<!-- open-swe-review-comment {"id":"f1",'
|
|
'"file_path":"a.py","start_line":1,'
|
|
'"end_line":1,"side":"RIGHT"} -->\n\nbug'
|
|
),
|
|
}
|
|
],
|
|
}
|
|
],
|
|
)
|
|
|
|
assert result[0]["github_review_comment_id"] == 11
|
|
assert result[0]["github_review_comment_ids"] == [11]
|
|
assert result[0]["github_review_thread_id"] == "THREAD_1"
|
|
assert result[0]["github_review_thread_ids"] == ["THREAD_1"]
|
|
replace.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_backfills_marker_from_graphql_app_login() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": None,
|
|
"github_review_thread_id": None,
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_1",
|
|
"is_resolved": False,
|
|
"is_outdated": False,
|
|
"comments": [
|
|
{
|
|
"id": 11,
|
|
"author": "open-swe",
|
|
"body": (
|
|
'<!-- open-swe-review-comment {"id":"f1",'
|
|
'"file_path":"a.py","start_line":1,'
|
|
'"end_line":1,"side":"RIGHT"} -->\n\nbug'
|
|
),
|
|
}
|
|
],
|
|
}
|
|
],
|
|
)
|
|
|
|
assert result[0]["github_review_comment_id"] == 11
|
|
assert result[0]["github_review_thread_id"] == "THREAD_1"
|
|
replace.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_duplicate_markers_require_all_threads_terminal() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": None,
|
|
"github_review_thread_id": None,
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
marker = (
|
|
'<!-- open-swe-review-comment {"id":"f1",'
|
|
'"file_path":"a.py","start_line":1,'
|
|
'"end_line":1,"side":"RIGHT"} -->\n\nbug'
|
|
)
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_OLD",
|
|
"is_resolved": False,
|
|
"is_outdated": True,
|
|
"comments": [{"id": 11, "author": "open-swe[bot]", "body": marker}],
|
|
},
|
|
{
|
|
"id": "THREAD_OPEN",
|
|
"is_resolved": False,
|
|
"is_outdated": False,
|
|
"comments": [{"id": 12, "author": "open-swe[bot]", "body": marker}],
|
|
},
|
|
],
|
|
)
|
|
|
|
assert result[0]["status"] == "open"
|
|
assert result[0]["github_review_comment_ids"] == [11, 12]
|
|
assert result[0]["github_review_thread_ids"] == ["THREAD_OLD", "THREAD_OPEN"]
|
|
replace.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_duplicate_markers_stay_open_when_some_threads_only_outdated() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": None,
|
|
"github_review_thread_id": None,
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
marker = (
|
|
'<!-- open-swe-review-comment {"id":"f1",'
|
|
'"file_path":"a.py","start_line":1,'
|
|
'"end_line":1,"side":"RIGHT"} -->\n\nbug'
|
|
)
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_OLD",
|
|
"is_resolved": False,
|
|
"is_outdated": True,
|
|
"comments": [{"id": 11, "author": "open-swe[bot]", "body": marker}],
|
|
},
|
|
{
|
|
"id": "THREAD_RESOLVED",
|
|
"is_resolved": True,
|
|
"is_outdated": False,
|
|
"comments": [{"id": 12, "author": "open-swe[bot]", "body": marker}],
|
|
},
|
|
],
|
|
)
|
|
|
|
assert result[0]["status"] == "open"
|
|
assert "last_reconciliation_note" not in result[0]
|
|
assert result[0]["github_resolved_thread_ids"] == ["THREAD_RESOLVED"]
|
|
assert result[0].get("github_thread_resolved") is not True
|
|
replace.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_ignores_spoofed_non_bot_marker() -> None:
|
|
findings = [
|
|
{
|
|
"id": "f1",
|
|
"status": "open",
|
|
"github_review_comment_id": None,
|
|
"github_review_thread_id": None,
|
|
}
|
|
]
|
|
replace = AsyncMock()
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_1",
|
|
"is_resolved": False,
|
|
"is_outdated": False,
|
|
"comments": [
|
|
{
|
|
"id": 11,
|
|
"author": "human",
|
|
"body": (
|
|
'<!-- open-swe-review-comment {"id":"f1",'
|
|
'"file_path":"a.py","start_line":1,'
|
|
'"end_line":1,"side":"RIGHT"} -->\n\nspoof'
|
|
),
|
|
}
|
|
],
|
|
}
|
|
],
|
|
)
|
|
|
|
assert result[0]["github_review_comment_id"] is None
|
|
assert result[0]["github_review_thread_id"] is None
|
|
replace.assert_not_awaited()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_reconcile_records_latest_human_reply_after_bot_comment() -> None:
|
|
findings = [{"id": "f1", "status": "open", "github_review_comment_id": 11}]
|
|
replace = AsyncMock()
|
|
|
|
with (
|
|
patch("agent.review.reconcile.list_findings", AsyncMock(return_value=findings)),
|
|
patch("agent.review.reconcile.replace_findings", replace),
|
|
):
|
|
result = await reconcile_findings_with_review_threads(
|
|
"tid",
|
|
[
|
|
{
|
|
"id": "THREAD_1",
|
|
"is_resolved": False,
|
|
"is_outdated": False,
|
|
"comments": [
|
|
{"id": 11, "author": "open-swe[bot]", "body": "bug"},
|
|
{
|
|
"id": 12,
|
|
"author": "human",
|
|
"body": "This is not valid because the caller already guards it.",
|
|
"created_at": "2026-05-26T10:00:00Z",
|
|
},
|
|
],
|
|
}
|
|
],
|
|
)
|
|
|
|
assert result[0]["github_review_thread_id"] == "THREAD_1"
|
|
assert result[0]["last_human_reply_author"] == "human"
|
|
assert "not valid" in result[0]["last_human_reply_body"]
|
|
replace.assert_awaited_once()
|