feat(python/robomp): added followup thread context to comment handling
- Normalized reviewer-bot matching by stripping a trailing `[bot]` suffix when resolving configured bot logins. - Fetched PR thread history for followup comments without directives and carried it through task execution. - Updated followup prompt rendering to include prior conversation context for handle_comment tasks.
This commit is contained in:
@@ -157,11 +157,16 @@ def route(
|
||||
return issue_key(repo, pr_number) # type: ignore[arg-type]
|
||||
|
||||
def _reviewer_bot_login(user: Mapping[str, Any] | None) -> str | None:
|
||||
"""Return the lowercased login if this user is a configured reviewer bot."""
|
||||
"""Return the normalized login if this user is a configured reviewer bot."""
|
||||
if not isinstance(user, Mapping):
|
||||
return None
|
||||
login = str(user.get("login") or "").lower()
|
||||
return login if login and login in reviewer_bots else None
|
||||
raw_login = str(user.get("login") or "").lower()
|
||||
if not raw_login:
|
||||
return None
|
||||
login = raw_login.removesuffix("[bot]")
|
||||
if login in reviewer_bots:
|
||||
return login
|
||||
return raw_login if raw_login in reviewer_bots else None
|
||||
|
||||
def _directive_kwargs(comment: Mapping[str, Any] | None, login: str | None, assoc: str | None) -> dict[str, Any]:
|
||||
"""Decide whether this comment is a directive (reviewer-bot OR maintainer-mention)."""
|
||||
|
||||
@@ -263,6 +263,7 @@ def followup_comment(
|
||||
workspace: Workspace,
|
||||
pr_status: str,
|
||||
pr_number: int | None = None,
|
||||
thread: tuple = (),
|
||||
) -> str:
|
||||
return render(
|
||||
_load("followup_comment.md"),
|
||||
@@ -271,6 +272,7 @@ def followup_comment(
|
||||
"issue": issue,
|
||||
"workspace": workspace,
|
||||
"comment": comment,
|
||||
"thread": _render_thread(thread),
|
||||
"state": {"pr_status": pr_status},
|
||||
"inbound": _inbound_scope(issue, pr_number),
|
||||
"origin": _origin_scope(issue),
|
||||
|
||||
@@ -2,6 +2,12 @@
|
||||
|
||||
Thread context: {{origin.description}}. PR state: `{{state.pr_status}}`.
|
||||
|
||||
## Prior conversation
|
||||
|
||||
{{thread}}
|
||||
|
||||
---
|
||||
|
||||
## New comment by @{{comment.author}} ({{comment.created_at}})
|
||||
|
||||
{{comment.body}}
|
||||
|
||||
@@ -666,8 +666,19 @@ async def handle_pr_conversation(
|
||||
slot_uid=slot_uid,
|
||||
natives_cache=sandbox.natives_cache,
|
||||
)
|
||||
directive = await _attach_thread(github, directive, repo_full, pr_number, is_pr=True)
|
||||
await run_task(task_kind="handle_comment", inputs=inputs, comment=comment, pr_number=pr_number, directive=directive)
|
||||
thread: tuple[ThreadMessage, ...] = ()
|
||||
if directive is None:
|
||||
thread = await _fetch_thread(github, repo_full, pr_number, is_pr=True)
|
||||
else:
|
||||
directive = await _attach_thread(github, directive, repo_full, pr_number, is_pr=True)
|
||||
await run_task(
|
||||
task_kind="handle_comment",
|
||||
inputs=inputs,
|
||||
comment=comment,
|
||||
pr_number=pr_number,
|
||||
directive=directive,
|
||||
thread=thread,
|
||||
)
|
||||
|
||||
|
||||
async def cleanup_workspace(
|
||||
|
||||
@@ -369,6 +369,7 @@ def _build_prompt(
|
||||
pr_number: int | None,
|
||||
review_payload: dict[str, Any] | None,
|
||||
directive: DirectiveInfo | None = None,
|
||||
thread: tuple[ThreadMessage, ...] = (),
|
||||
resuming: bool = False,
|
||||
) -> str:
|
||||
if task_kind == "triage_issue":
|
||||
@@ -412,6 +413,7 @@ def _build_prompt(
|
||||
comment=comment,
|
||||
pr_status=pr_status,
|
||||
pr_number=pr_number,
|
||||
thread=thread,
|
||||
)
|
||||
if task_kind == "handle_review":
|
||||
assert review_payload is not None
|
||||
@@ -652,6 +654,7 @@ async def run_task(
|
||||
pr_number: int | None = None,
|
||||
review_payload: dict[str, Any] | None = None,
|
||||
directive: DirectiveInfo | None = None,
|
||||
thread: tuple[ThreadMessage, ...] = (),
|
||||
) -> str | None:
|
||||
"""Async wrapper that runs the synchronous RPC driver on a worker thread."""
|
||||
loop = asyncio.get_running_loop()
|
||||
@@ -679,6 +682,7 @@ async def run_task(
|
||||
pr_number=pr_number,
|
||||
review_payload=review_payload,
|
||||
directive=directive,
|
||||
thread=thread,
|
||||
resuming=resuming,
|
||||
)
|
||||
try:
|
||||
|
||||
@@ -592,7 +592,7 @@ def test_route_reviewer_bot_comment_is_directive_without_mention() -> None:
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "chatgpt-codex-connector", "type": "Bot"},
|
||||
"user": {"login": "chatgpt-codex-connector[bot]", "type": "Bot"},
|
||||
"body": "Found two issues in the diff: ...",
|
||||
},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
@@ -616,7 +616,7 @@ def test_route_reviewer_bot_review_comment_is_directive() -> None:
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "chatgpt-codex-connector", "type": "Bot"},
|
||||
"user": {"login": "chatgpt-codex-connector[bot]", "type": "Bot"},
|
||||
"body": "This branch leaks memory.",
|
||||
},
|
||||
"pull_request": {"number": 50, "user": {"login": BOT}},
|
||||
|
||||
@@ -102,6 +102,27 @@ def test_directive_prompt_embeds_thread_and_directive_body() -> None:
|
||||
assert "PR #1080 is open" in out
|
||||
|
||||
|
||||
def test_followup_comment_prompt_embeds_thread_context() -> None:
|
||||
thread = (
|
||||
ThreadMessage(kind="pr_body", author="roboomp", body="PR body", created_at=""),
|
||||
ThreadMessage(kind="comment", author="can1357", body="prior request", created_at="2026-05-01T10:00:00Z"),
|
||||
)
|
||||
out = persona.followup_comment(
|
||||
repo=_Repo(),
|
||||
issue=_Issue(),
|
||||
comment=_Comment(body="current request"),
|
||||
workspace=_Workspace(),
|
||||
pr_status="PR #1080 is open",
|
||||
pr_number=1080,
|
||||
thread=thread,
|
||||
)
|
||||
|
||||
assert "Prior conversation" in out
|
||||
assert "PR body" in out
|
||||
assert "prior request" in out
|
||||
assert "current request" in out
|
||||
|
||||
|
||||
def test_kickoff_directive_prompt_embeds_thread_and_classify_instruction() -> None:
|
||||
thread = (ThreadMessage(kind="issue_body", author="alice", body="failing on macos", created_at=""),)
|
||||
out = persona.kickoff_directive(
|
||||
|
||||
@@ -1610,7 +1610,7 @@ async def test_handle_pr_conversation_unmapped_bot_pr_uses_pr_branch(
|
||||
settings: Settings, tmp_path: Path, stub_run_task, monkeypatch
|
||||
) -> None:
|
||||
from robomp import tasks
|
||||
from robomp.github_client import IssueInfo, PullRequestInfo, RepoInfo
|
||||
from robomp.github_client import CommentInfo, IssueInfo, PullRequestInfo, RepoInfo
|
||||
|
||||
sandbox = _RecordingSandbox(tmp_path)
|
||||
db = get_database(settings.sqlite_path)
|
||||
@@ -1652,9 +1652,27 @@ async def test_handle_pr_conversation_unmapped_bot_pr_uses_pr_branch(
|
||||
assert number == 900
|
||||
return pr_issue
|
||||
|
||||
async def _list_comments(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return [CommentInfo(id=77, author="can1357", body="prior PR context", created_at="2026-05-14T00:00:00Z")]
|
||||
|
||||
async def _list_review_comments(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return []
|
||||
|
||||
async def _list_pr_reviews(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return []
|
||||
|
||||
monkeypatch.setattr(GitHubClient, "get_pull_request", _get_pull_request)
|
||||
monkeypatch.setattr(GitHubClient, "get_repo", _get_repo)
|
||||
monkeypatch.setattr(GitHubClient, "get_issue", _get_issue)
|
||||
monkeypatch.setattr(GitHubClient, "list_comments", _list_comments)
|
||||
monkeypatch.setattr(GitHubClient, "list_review_comments", _list_review_comments)
|
||||
monkeypatch.setattr(GitHubClient, "list_pr_reviews", _list_pr_reviews)
|
||||
|
||||
payload = {
|
||||
"action": "created",
|
||||
@@ -1677,6 +1695,10 @@ async def test_handle_pr_conversation_unmapped_bot_pr_uses_pr_branch(
|
||||
assert call["task_kind"] == "handle_comment"
|
||||
assert call["pr_number"] == 900
|
||||
assert call["inputs"].issue.is_pull_request is True
|
||||
assert [(m.kind, m.author, m.body) for m in call["thread"]] == [
|
||||
("pr_body", settings.bot_login, "PR body"),
|
||||
("comment", "can1357", "prior PR context"),
|
||||
]
|
||||
assert sandbox.ensure_calls[0]["number"] == 900
|
||||
assert sandbox.ensure_calls[0]["existing_branch"] == "farm/abc12345/fix-flaky-parser"
|
||||
row = db.get_issue("octo/widget#900")
|
||||
@@ -1690,7 +1712,7 @@ async def test_handle_pr_conversation_repairs_missing_pr_mapping_from_branch(
|
||||
settings: Settings, tmp_path: Path, stub_run_task, monkeypatch
|
||||
) -> None:
|
||||
from robomp import tasks
|
||||
from robomp.github_client import IssueInfo, PullRequestInfo, RepoInfo
|
||||
from robomp.github_client import CommentInfo, IssueInfo, PullRequestInfo, RepoInfo
|
||||
|
||||
sandbox = _RecordingSandbox(tmp_path)
|
||||
db = get_database(settings.sqlite_path)
|
||||
@@ -1709,6 +1731,16 @@ async def test_handle_pr_conversation_repairs_missing_pr_mapping_from_branch(
|
||||
labels=(),
|
||||
is_pull_request=False,
|
||||
)
|
||||
pr_issue = IssueInfo(
|
||||
repo="octo/widget",
|
||||
number=900,
|
||||
title="Fix flaky parser",
|
||||
body="PR body",
|
||||
state="open",
|
||||
author=settings.bot_login,
|
||||
labels=(),
|
||||
is_pull_request=True,
|
||||
)
|
||||
pr_info = PullRequestInfo(
|
||||
repo="octo/widget",
|
||||
number=900,
|
||||
@@ -1731,12 +1763,33 @@ async def test_handle_pr_conversation_repairs_missing_pr_mapping_from_branch(
|
||||
|
||||
async def _get_issue(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 42
|
||||
return issue
|
||||
if number == 42:
|
||||
return issue
|
||||
if number == 900:
|
||||
return pr_issue
|
||||
raise AssertionError(number)
|
||||
|
||||
async def _list_comments(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return [CommentInfo(id=78, author="can1357", body="prior PR context", created_at="2026-05-14T00:00:00Z")]
|
||||
|
||||
async def _list_review_comments(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return []
|
||||
|
||||
async def _list_pr_reviews(self, repo_full: str, number: int):
|
||||
assert repo_full == "octo/widget"
|
||||
assert number == 900
|
||||
return []
|
||||
|
||||
monkeypatch.setattr(GitHubClient, "get_pull_request", _get_pull_request)
|
||||
monkeypatch.setattr(GitHubClient, "get_repo", _get_repo)
|
||||
monkeypatch.setattr(GitHubClient, "get_issue", _get_issue)
|
||||
monkeypatch.setattr(GitHubClient, "list_comments", _list_comments)
|
||||
monkeypatch.setattr(GitHubClient, "list_review_comments", _list_review_comments)
|
||||
monkeypatch.setattr(GitHubClient, "list_pr_reviews", _list_pr_reviews)
|
||||
|
||||
payload = {
|
||||
"action": "created",
|
||||
@@ -1758,6 +1811,10 @@ async def test_handle_pr_conversation_repairs_missing_pr_mapping_from_branch(
|
||||
call = stub_run_task[0]
|
||||
assert call["inputs"].issue.number == 42
|
||||
assert call["pr_number"] == 900
|
||||
assert [(m.kind, m.author, m.body) for m in call["thread"]] == [
|
||||
("pr_body", settings.bot_login, "PR body"),
|
||||
("comment", "can1357", "prior PR context"),
|
||||
]
|
||||
assert sandbox.ensure_calls[0]["number"] == 42
|
||||
assert sandbox.ensure_calls[0]["existing_branch"] == branch
|
||||
row = db.get_issue("octo/widget#42")
|
||||
|
||||
Reference in New Issue
Block a user