WYL-51: re-trigger assignee on REJECT verdict
After _advance_awaiting_judge kicks an issue back to in_progress on a REJECT, post a short followup comment that @-mentions the agent assignee with the verdict, top 2-3 failure reasons, and a retry prompt. Corner cases handled: - assignee_type != 'agent' (member or unset) → skip silently - ACCEPT branch → no notification - notification failure → logged, round still completes (non-blocking) New helpers: _extract_rejection_reasons, _build_rejection_followup, _notify_assignee_on_reject. +12 tests (5 for _extract_rejection_reasons, 7 for the notify path). Total: 66 passed.
This commit is contained in:
@@ -258,6 +258,123 @@ def _build_debater_comment(
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
def _extract_rejection_reasons(judge_reply: str) -> list[str]:
|
||||
"""Extract up to 3 failure reasons from a REJECT verdict text.
|
||||
|
||||
Prefers explicit bullet / numbered list items; falls back to the first
|
||||
two sentences of the reasoning block.
|
||||
"""
|
||||
# Strip the VERDICT line itself
|
||||
text = _VERDICT_RE.sub("", judge_reply, count=1).strip()
|
||||
|
||||
bullets: list[str] = []
|
||||
for line in text.splitlines():
|
||||
stripped = line.strip()
|
||||
if re.match(r"^[-*•]\s+\S", stripped) or re.match(r"^\d+[.)]\s+\S", stripped):
|
||||
reason = re.sub(r"^[-*•\d.)\s]+", "", stripped).strip()
|
||||
if reason:
|
||||
bullets.append(reason)
|
||||
if len(bullets) >= 3:
|
||||
break
|
||||
|
||||
if bullets:
|
||||
return bullets[:3]
|
||||
|
||||
# Fallback: first two non-empty sentences
|
||||
sentences = [s.strip() for s in re.split(r"(?<=[.!?])\s+", text) if s.strip()]
|
||||
return sentences[:2]
|
||||
|
||||
|
||||
def _build_rejection_followup(
|
||||
assignee_name: str,
|
||||
assignee_agent_id: str,
|
||||
judge_reply: str,
|
||||
) -> str:
|
||||
"""Short comment @-mentioning the assignee with the REJECT summary."""
|
||||
mention = (
|
||||
f"[@{assignee_name}](mention://agent/{assignee_agent_id})"
|
||||
if assignee_agent_id
|
||||
else f"@{assignee_name}"
|
||||
)
|
||||
reasons = _extract_rejection_reasons(judge_reply)
|
||||
if reasons:
|
||||
reason_block = "\n".join(f"- {r}" for r in reasons)
|
||||
body = f"Top issues:\n{reason_block}"
|
||||
else:
|
||||
body = judge_reply[:400].strip()
|
||||
|
||||
return (
|
||||
f"{mention} — VERDICT: REJECT\n\n"
|
||||
f"{body}\n\n"
|
||||
"Please address these issues and resubmit."
|
||||
)
|
||||
|
||||
|
||||
def _notify_assignee_on_reject(
|
||||
round_: Round,
|
||||
judge_reply: str,
|
||||
client: MulticaClient,
|
||||
logger: logging.Logger,
|
||||
) -> None:
|
||||
"""Post a rejection followup comment @-mentioning the original assignee.
|
||||
|
||||
Only fires when ``assignee_type == 'agent'``. Any failure is logged and
|
||||
swallowed — round completion must not depend on this side-effect.
|
||||
"""
|
||||
try:
|
||||
issue = client.get_issue(round_.issue_id)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"round %s: cannot fetch issue for reject notification: %s",
|
||||
round_.round_id, exc,
|
||||
)
|
||||
return
|
||||
|
||||
assignee_type = issue.get("assignee_type", "")
|
||||
assignee_id = issue.get("assignee_id", "")
|
||||
|
||||
if assignee_type != "agent" or not assignee_id:
|
||||
logger.debug(
|
||||
"round %s: assignee_type=%r — skipping reject notification",
|
||||
round_.round_id, assignee_type,
|
||||
)
|
||||
return
|
||||
|
||||
# Resolve the agent name from their ID
|
||||
try:
|
||||
agents = client.list_agents()
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"round %s: cannot list agents for reject notification: %s",
|
||||
round_.round_id, exc,
|
||||
)
|
||||
return
|
||||
|
||||
assignee_name = next(
|
||||
(a.get("name", "") for a in agents if a.get("id") == assignee_id),
|
||||
"",
|
||||
)
|
||||
if not assignee_name:
|
||||
logger.warning(
|
||||
"round %s: assignee agent %s not found in workspace — skipping reject notification",
|
||||
round_.round_id, assignee_id,
|
||||
)
|
||||
return
|
||||
|
||||
body = _build_rejection_followup(assignee_name, assignee_id, judge_reply)
|
||||
try:
|
||||
client.post_comment(round_.issue_id, body)
|
||||
logger.info(
|
||||
"round %s: reject notification posted to @%s",
|
||||
round_.round_id, assignee_name,
|
||||
)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"round %s: cannot post reject notification: %s",
|
||||
round_.round_id, exc,
|
||||
)
|
||||
|
||||
|
||||
def _build_judge_comment(
|
||||
judge_agent_id: str,
|
||||
issue_title: str,
|
||||
@@ -508,6 +625,10 @@ def _advance_awaiting_judge(
|
||||
)
|
||||
return # do NOT mark done; retry next cycle
|
||||
|
||||
# On REJECT: re-trigger the original assignee via a short followup comment.
|
||||
if verdict == "REJECT":
|
||||
_notify_assignee_on_reject(round_, judge_reply, client, logger)
|
||||
|
||||
phase = "accepted" if verdict == "ACCEPT" else "rejected"
|
||||
queue.update_phase(round_.round_id, phase)
|
||||
queue.update_status(round_.round_id, "done")
|
||||
|
||||
@@ -17,6 +17,7 @@ from coordinator.orchestrator import (
|
||||
_advance_awaiting_judge,
|
||||
_collect_debater_replies,
|
||||
_collect_judge_reply,
|
||||
_extract_rejection_reasons,
|
||||
_find_commit_url,
|
||||
_parse_verdict,
|
||||
_start_round,
|
||||
@@ -734,3 +735,216 @@ def test_orchestrate_pending_advances_running(tmp_path):
|
||||
|
||||
assert r.status == "done"
|
||||
assert r.phase == "rejected"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _extract_rejection_reasons
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_extract_reasons_from_bullets():
|
||||
text = "VERDICT: REJECT\n\n- Missing tests\n- No error handling\n- Bad performance"
|
||||
reasons = _extract_rejection_reasons(text)
|
||||
assert reasons == ["Missing tests", "No error handling", "Bad performance"]
|
||||
|
||||
|
||||
def test_extract_reasons_from_numbered_list():
|
||||
text = "VERDICT: REJECT\n\n1. Unit tests missing\n2. Docs incomplete"
|
||||
reasons = _extract_rejection_reasons(text)
|
||||
assert reasons == ["Unit tests missing", "Docs incomplete"]
|
||||
|
||||
|
||||
def test_extract_reasons_capped_at_three():
|
||||
text = "VERDICT: REJECT\n\n- R1\n- R2\n- R3\n- R4\n- R5"
|
||||
assert len(_extract_rejection_reasons(text)) == 3
|
||||
|
||||
|
||||
def test_extract_reasons_fallback_to_sentences():
|
||||
text = "VERDICT: REJECT\n\nImplementation is broken. Tests are absent. Nothing works."
|
||||
reasons = _extract_rejection_reasons(text)
|
||||
assert len(reasons) <= 2
|
||||
assert any("broken" in r.lower() for r in reasons)
|
||||
|
||||
|
||||
def test_extract_reasons_empty_text():
|
||||
reasons = _extract_rejection_reasons("VERDICT: REJECT")
|
||||
assert isinstance(reasons, list)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _advance_awaiting_judge — REJECT assignee notification (WYL-51)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _judge_round_with_agent_assignee(
|
||||
tmp_path: Path,
|
||||
*,
|
||||
assignee_id: str = "agent-worker",
|
||||
assignee_name: str = "Senior Developer",
|
||||
) -> tuple[Round, DebateQueue, FakeClient]:
|
||||
"""_judge_round helper pre-configured with an agent assignee."""
|
||||
r, q, client = _judge_round(tmp_path)
|
||||
client.issue["assignee_type"] = "agent"
|
||||
client.issue["assignee_id"] = assignee_id
|
||||
client.agents.append({"name": assignee_name, "id": assignee_id})
|
||||
return r, q, client
|
||||
|
||||
|
||||
def test_reject_posts_followup_with_assignee_mention(tmp_path):
|
||||
"""REJECT path: a followup comment is posted that @-mentions the agent assignee."""
|
||||
r, q, client = _judge_round_with_agent_assignee(tmp_path)
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: REJECT\n\n- Missing tests\n- No error handling",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
assert r.status == "done"
|
||||
assert r.phase == "rejected"
|
||||
followup = next(
|
||||
(c for c in client.posted_comments if "mention://agent/agent-worker" in c),
|
||||
None,
|
||||
)
|
||||
assert followup is not None, "Expected followup comment with @-mention of assignee"
|
||||
assert "REJECT" in followup
|
||||
|
||||
|
||||
def test_reject_followup_contains_verdict_reasoning(tmp_path):
|
||||
"""REJECT followup contains extracted failure reasons from the judge reply."""
|
||||
r, q, client = _judge_round_with_agent_assignee(tmp_path)
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": (
|
||||
"VERDICT: REJECT\n\n"
|
||||
"- Unit tests are missing\n"
|
||||
"- Error handling is incomplete\n"
|
||||
"- Performance is unacceptable"
|
||||
),
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
followup = next(
|
||||
(c for c in client.posted_comments if "mention://agent/agent-worker" in c),
|
||||
None,
|
||||
)
|
||||
assert followup is not None
|
||||
assert "tests" in followup.lower() or "error" in followup.lower() or "performance" in followup.lower()
|
||||
|
||||
|
||||
def test_reject_followup_includes_retry_request(tmp_path):
|
||||
"""REJECT followup asks the assignee to retry."""
|
||||
r, q, client = _judge_round_with_agent_assignee(tmp_path)
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: REJECT\n\nDoes not meet acceptance criteria.",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
followup = next(
|
||||
(c for c in client.posted_comments if "mention://agent/agent-worker" in c),
|
||||
None,
|
||||
)
|
||||
assert followup is not None
|
||||
assert "resubmit" in followup.lower() or "retry" in followup.lower()
|
||||
|
||||
|
||||
def test_reject_no_followup_for_member_assignee(tmp_path):
|
||||
"""REJECT path: no followup when assignee_type == 'member' (leave to human)."""
|
||||
r, q, client = _judge_round(tmp_path)
|
||||
client.issue["assignee_type"] = "member"
|
||||
client.issue["assignee_id"] = "user-123"
|
||||
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: REJECT\n\nNeeds more work.",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
assert r.status == "done"
|
||||
assert r.phase == "rejected"
|
||||
assert all("mention://" not in c for c in client.posted_comments)
|
||||
|
||||
|
||||
def test_reject_no_followup_when_no_assignee(tmp_path):
|
||||
"""REJECT path: no followup when issue has no assignee set."""
|
||||
r, q, client = _judge_round(tmp_path)
|
||||
# Default FakeClient issue has no assignee fields
|
||||
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: REJECT\n\nNo work done.",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
assert r.status == "done"
|
||||
assert r.phase == "rejected"
|
||||
assert len(client.posted_comments) == 0
|
||||
|
||||
|
||||
def test_reject_notification_failure_does_not_block_round_completion(tmp_path):
|
||||
"""REJECT followup: round completes even if posting the notification fails."""
|
||||
r, q, client = _judge_round_with_agent_assignee(tmp_path)
|
||||
|
||||
original_post = FakeClient.post_comment
|
||||
call_count = [0]
|
||||
|
||||
def post_with_failure(self, issue_id, content):
|
||||
call_count[0] += 1
|
||||
if call_count[0] > 1:
|
||||
raise RuntimeError("API error on notification")
|
||||
return original_post(self, issue_id, content)
|
||||
|
||||
FakeClient.post_comment = post_with_failure
|
||||
try:
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: REJECT\n\nFails acceptance criteria.",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
assert r.status == "done", "Round must complete even if notification fails"
|
||||
assert r.phase == "rejected"
|
||||
finally:
|
||||
FakeClient.post_comment = original_post
|
||||
|
||||
|
||||
def test_accept_does_not_post_rejection_followup(tmp_path):
|
||||
"""ACCEPT path: no assignee followup comment is posted."""
|
||||
r, q, client = _judge_round_with_agent_assignee(tmp_path)
|
||||
|
||||
client.comments.append({
|
||||
"id": "verdict1",
|
||||
"content": "VERDICT: ACCEPT\n\nExcellent work!",
|
||||
"author_id": "agent-judge",
|
||||
"author_type": "agent",
|
||||
"created_at": _utcnow(),
|
||||
})
|
||||
|
||||
_advance_awaiting_judge(r, client, q, FakeConfig(), _logger)
|
||||
|
||||
assert r.status == "done"
|
||||
assert r.phase == "accepted"
|
||||
# No followup @-mention to the assignee
|
||||
assert all("mention://agent/agent-worker" not in c for c in client.posted_comments)
|
||||
|
||||
Reference in New Issue
Block a user