External/roboco
1
0
Fork 0
mirror of https://github.com/rennf93/roboco.git synced 2026-08-03 07:23:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

feat(content): structured PR-review findings + generated GitHub comment

Browse Source
This commit is contained in:
Renn F
2026-06-21 03:47:17 +02:00
parent b97465a631
commit 11c1d9eee7
7 changed files with 171 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
agents/prompts/roles/pr_reviewer.md
+2 -2
View File
@@ -22,7 +22,7 @@ The PR is from an outside contributor: its code is **untrusted**. Until a human
|---|---|---|
| `give_me_work()` | Returns an external-PR review task or `idle`. | None. |
| `claim_pr_review(task_id)` | Claims the review task and starts it. `pending → claimed → in_progress`. Returns the PR diff inline. | Task is an `external_pr` review task in `pending`. |
| `post_pr_review(task_id, ...)` | Posts ONE complete change-request to the PR and finishes the review task. `in_progress → completed`. | Task claimed by you; findings cover every relevant criterion. |
| `post_pr_review(task_id, body, findings=[...])` | Posts ONE complete change-request and finishes the review. `in_progress → completed`. `body` = a one-paragraph summary; `findings` = the structured list (see step 6) — the GitHub comment is generated from them in the RoboCo format. | Task claimed by you; findings cover every relevant criterion. |
| `note(text, scope?)` | Journal entry. Record your reasoning. | None. |
| `evidence(task_id)` | Re-fetch the PR diff if you need more detail. | None. |
| `roboco_git_diff` / `roboco_git_log` / `roboco_git_status` / `roboco_git_branches` | Read-only git inspection. | None. |
@@ -35,7 +35,7 @@ The PR is from an outside contributor: its code is **untrusted**. Until a human
3. Review the diff **read-only**. Do NOT run the contributor's code unless the PR is human-confirmed.
4. For each acceptance criterion and each correctness/security/quality concern, find the specific evidence (file/line) and form a concrete, actionable finding.
5. `note(scope='learning', ...)` capturing what the review surfaced.
6. `post_pr_review(task_id, ...)` → one complete change-request, per-criterion findings, each referencing file + line + expected vs actual.
6. `post_pr_review(task_id, body="<one-paragraph summary>", findings=[...])` — supply **structured** findings, one object per issue: `{"file": "path", "line": 42, "severity": "blocker|major|minor|nit", "expected": "...", "actual": "..."}`. The GitHub comment is generated in the RoboCo format (summary + findings table + verdict); do not hand-format the body.
## Anti-patterns
roboco/api/routes/v1/flow_pr_reviewer.py
+1 -1
View File
@@ -59,7 +59,7 @@ async def post_pr_review(
choreographer: _ChoreographerDep,
) -> dict:
env = await choreographer.post_pr_review(
x_agent_id, body.task_id, body.body, body.event
x_agent_id, body.task_id, body.body, body.event, body.findings
)
return envelope_to_response(env, request)
roboco/api/schemas/v1/flow.py
+9
View File
@@ -1,5 +1,6 @@
"""Request schemas for /api/v1/flow/* intent verbs."""
from typing import Any
from uuid import UUID
from pydantic import BaseModel, Field, field_validator
@@ -133,6 +134,14 @@ class PostPrReviewRequest(BaseModel):
task_id: UUID
body: str = Field(..., min_length=1)
event: str = "REQUEST_CHANGES"
findings: list[dict[str, Any]] = Field(
default_factory=list,
description=(
"Structured per-criterion findings — each {file, line?, severity "
"(blocker|major|minor|nit), expected, actual}. When provided, the "
"GitHub comment is generated from them in the RoboCo format."
),
)
class ClaimGateReviewRequest(BaseModel):
roboco/mcp/flow_server.py
+16 -4
View File
@@ -365,16 +365,28 @@ def claim_pr_review(task_id: str) -> dict[str, Any]:
def post_pr_review(
task_id: str, body: str, event: str = "REQUEST_CHANGES"
task_id: str,
body: str,
event: str = "REQUEST_CHANGES",
findings: list[dict[str, Any]] | None = None,
) -> dict[str, Any]:
"""PR reviewer: post ONE complete change-request to the PR and finish the task.
body: the full review (per-criterion findings). event: REQUEST_CHANGES
(default), APPROVE, or COMMENT. Requires a journal:learning entry first.
body: a one-paragraph summary. findings: the per-criterion list — each
{file, line?, severity (blocker|major|minor|nit), expected, actual}. When
findings are given, the GitHub comment is GENERATED in the RoboCo format
(summary + a findings table + verdict) — do not hand-format it in body.
event: REQUEST_CHANGES (default), APPROVE, or COMMENT. Requires a
journal:learning entry first.
"""
return _post(
_role_path("post_pr_review"),
{"task_id": task_id, "body": body, "event": event},
{
"task_id": task_id,
"body": body,
"event": event,
"findings": findings or [],
},
)
roboco/services/gateway/choreographer/pr_review.py
+85 -25
View File
@@ -21,6 +21,8 @@ import structlog
from roboco.foundation.policy import lifecycle as spec_module
from roboco.foundation.policy import tracing as _tr
from roboco.foundation.policy.content import ContentValidationError, validate_content
from roboco.services.content_notes import apply_structured_note
from roboco.services.gateway.envelope import Envelope
if TYPE_CHECKING:
@@ -115,12 +117,83 @@ class PRReviewerMixin(_Base):
context_briefing=briefing,
).with_introspection(task=t, role=role_str)
@staticmethod
def _build_pr_review_content(
body: str, findings: list[dict[str, Any]], event: str
) -> Any:
"""Validate structured findings into a PrReviewContent, or an Envelope.
The reviewer supplies a summary (``body``) + structured ``findings``; the
canonical GitHub comment is generated from them. ``event`` maps to the
verdict (APPROVE → approved, else changes_requested).
"""
verdict = "approved" if event == "APPROVE" else "changes_requested"
try:
return validate_content(
"pr_review",
{"summary": body, "findings": findings, "verdict": verdict},
)
except ContentValidationError as exc:
return Envelope.invalid_state(
message=f"malformed PR-review findings: {exc.field} — {exc.reason}",
remediate=(
"each finding needs file + expected + actual (line + severity "
"optional); re-call post_pr_review with structured findings"
),
)
def _resolve_post_body(
self, t: Any, body: str, findings: list[dict[str, Any]] | None, event: str
) -> Any:
"""The GitHub comment body: the canonical render when findings are given
(and stored structured), else the free-text body. Envelope on malformed
findings."""
if not findings:
return body
structured = self._build_pr_review_content(body, findings, event)
if isinstance(structured, Envelope):
return structured
apply_structured_note(t, "pr_review", structured)
return structured.render_markdown()
async def _post_review_side_effects(
self,
t: Any,
slug: str | None,
pr_number: int | None,
post_body: str,
event: str,
task_id: UUID,
) -> None:
"""Post the review to GitHub + surface it to the CEO (both best-effort)."""
if slug and pr_number:
try:
await self.git.post_pr_review(slug, pr_number, post_body, event=event)
except Exception:
logger.exception(
"post_pr_review GitHub post failed", task_id=str(task_id)
)
if pr_number:
try:
from roboco.services.notification import NotificationService
await NotificationService().send_external_pr_reviewed_notification(
task_id=str(task_id),
pr_number=pr_number,
pr_url=str(getattr(t, "pr_url", "") or ""),
)
except Exception:
logger.exception(
"post_pr_review CEO notify failed", task_id=str(task_id)
)
async def post_pr_review(
self,
reviewer_agent_id: UUID,
task_id: UUID,
body: str,
event: str = "REQUEST_CHANGES",
findings: list[dict[str, Any]] | None = None,
) -> Envelope:
"""Post ONE change-request to the PR and finish the review task.
@@ -144,6 +217,14 @@ class PRReviewerMixin(_Base):
agent, role_str, briefing, spec_ctx = pre
slug = await self._project_slug_for(t)
pr_number = t.pr_number
post_body = self._resolve_post_body(t, body, findings, event)
if isinstance(post_body, Envelope):
return await self._emit_rejection(
post_body.with_introspection(task=t, role=role_str),
agent_id=reviewer_agent_id,
task_id=task_id,
verb="post_pr_review",
)
runner = self._verb_runner()
try:
t = await runner.run_intent("post_pr_review", t, agent, spec_ctx)
@@ -151,31 +232,10 @@ class PRReviewerMixin(_Base):
return await self._runner_failure(
exc, t, role_str, briefing, reviewer_agent_id, task_id, "post_pr_review"
)
# GitHub side-effect AFTER the DB transition (a2a.send pattern). Best-
# effort: a posting failure is logged, not rolled back — the review task
# is complete; a missed post can be re-driven manually.
if slug and pr_number:
try:
await self.git.post_pr_review(slug, pr_number, body, event=event)
except Exception:
logger.exception(
"post_pr_review GitHub post failed", task_id=str(task_id)
)
# Surface the review to the CEO as an actionable decision (supersede /
# dismiss). The reviewer is read-only with no notify verb, so the server
# emits it. Best-effort — a notify failure must not fail the review.
if pr_number:
try:
from roboco.services.notification import NotificationService
await NotificationService().send_external_pr_reviewed_notification(
task_id=str(task_id),
pr_number=pr_number,
pr_url=str(getattr(t, "pr_url", "") or ""),
)
except Exception:
logger.exception(
"post_pr_review CEO notify failed", task_id=str(task_id)
# Side-effects AFTER the DB transition (a2a.send pattern), both best-
# effort: post the canonical review to GitHub + surface it to the CEO.
await self._post_review_side_effects(
t, slug, pr_number, post_body, event, task_id
)
return Envelope.ok(
status=str(t.status),
roboco/services/task.py
+4
View File
@@ -3649,7 +3649,11 @@ class TaskService(BaseService):
``qa_notes`` / ``dev_notes``. Structured per-line findings arrive via the
reviewer verb; until then the free-text summary + issues are captured.
Best-effort: a too-trivial review body must never block the transition.
Skips when a structured PrReviewContent is already stored (the
post_pr_review verb with findings does that itself).
"""
if (task.notes_structured or {}).get("pr_review"):
return
body = (summary or "").strip()
if issues:
bullets = "\n".join(f"- {i}" for i in issues if i and i.strip())
tests/unit/gateway/test_pr_review_structured.py
+53
View File
@@ -0,0 +1,53 @@
"""post_pr_review structured-findings helper.
The reviewer supplies a summary + structured findings; the verb generates the
canonical GitHub comment from them and rejects malformed findings.
"""
from __future__ import annotations
from roboco.foundation.policy.content import PrReviewContent
from roboco.services.gateway.choreographer.pr_review import PRReviewerMixin
from roboco.services.gateway.envelope import Envelope
_build = PRReviewerMixin._build_pr_review_content
def test_valid_findings_build_pr_review_content() -> None:
content = _build(
"The 422 path is unguarded.",
[
{
"file": "roboco/services/git.py",
"line": 42,
"severity": "blocker",
"expected": "retry as COMMENT",
"actual": "raises",
}
],
"REQUEST_CHANGES",
)
assert isinstance(content, PrReviewContent)
assert content.verdict.value == "changes_requested"
md = content.render_markdown()
assert "## Findings" in md
assert "`roboco/services/git.py`" in md
assert "blocker" in md
def test_approve_event_maps_to_approved_verdict() -> None:
content = _build("All criteria met; clean diff.", [], "APPROVE")
assert isinstance(content, PrReviewContent)
assert content.verdict.value == "approved"
def test_malformed_findings_return_envelope() -> None:
# A finding missing the required `actual` field is rejected with remediation.
result = _build(
"Something is off here.",
[{"file": "a.py", "severity": "minor", "expected": "x"}],
"REQUEST_CHANGES",
)
assert isinstance(result, Envelope)
assert result.error is not None
assert "finding" in (result.remediate or "").lower()