Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/agent_harness/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,34 @@ def is_disk_exhaustion(detail: str) -> bool:
"""


def review_reason(verdict_text: str, limit: int = 1200) -> str:
"""The reviewer's verdict, kept from the end rather than the beginning.

`last_error` is what a retry is told and what an operator reads, and it
used to be the first 500 characters of the verdict. The rubric asks for
"what I verified" first and "why" last, so those 500 characters were the
list of things the reviewer was *happy* with, and the objection — the only
part anyone can act on — was cut off.

Measured: two items were rejected twice for the same fault after being
told, both times, only the preamble praising the parts that were fine.

So the tail wins when something has to go. The head is a summary of the
diff, which the reader already has; the tail is the decision.
"""
text = verdict_text.strip()
if len(text) <= limit:
return text
# Prefer starting at a section boundary, so a retry is not handed half a
# sentence about something it cannot see the start of.
tail = text[-limit:]
for marker in ("\n3. ", "\n2. ", "\n\n"):
cut = tail.find(marker)
if 0 <= cut < limit // 2:
return "…" + tail[cut:].strip()
return "…" + tail.strip()


def review_context(repo: Path, diff: str, budget: int) -> str:
"""The touched files as they now stand, for the reviewer.

Expand Down Expand Up @@ -2413,7 +2441,7 @@ def _review_stage(
if outcome.pr_url and self.github is not None:
self._record_verdict(record, outcome.pr_url, verdict, verdict_text)
if verdict != APPROVED:
outcome.reason = f"review rejected: {verdict_text.strip()[:500]}"
outcome.reason = f"review rejected: {review_reason(verdict_text)}"
outcome.stop = Stop(REFUSED, REVIEW_REJECTED, detail=outcome.reason)
return outcome

Expand Down
3 changes: 2 additions & 1 deletion src/agent_harness/session_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@
#: items were rejected for exactly the artefacts those fixes had already
#: retired — in the other file (#167).
from .executor import REVIEW_PROMPT as REVIEW_PROMPT # noqa: E402
from .executor import review_reason # noqa: E402


@dataclass
Expand Down Expand Up @@ -647,7 +648,7 @@ def _execute(self, record: WorkRecord) -> Outcome:
self._record_verdict(record, outcome.pr_url, verdict, verdict_text)

if verdict != APPROVED:
outcome.reason = f"review rejected: {verdict_text.strip()[:500]}"
outcome.reason = f"review rejected: {review_reason(verdict_text)}"
outcome.stop = Stop(REFUSED, REVIEW_REJECTED, detail=outcome.reason)
return outcome

Expand Down
34 changes: 34 additions & 0 deletions tests/test_reviewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,37 @@ def call(self, _role: str, messages: Any, **_: Any) -> Any:

assert "hello harness" in seen["prompt"], "the reviewer was not shown the committed work"
assert "-hello world" in seen["prompt"], "nor what it replaced"


def test_the_kept_reason_is_the_objection_not_the_praise() -> None:
"""`last_error` is what a retry is told, and it kept the wrong end.

The rubric asks for "what I verified" first and "why" last. Keeping the
first 500 characters therefore kept the list of things the reviewer was
happy with and cut off the objection — the only part anyone can act on.

Measured: two items were rejected twice for the same fault, each retry
having been told only the preamble praising the parts that were fine.
"""
from agent_harness.executor import review_reason

verdict = (
"REJECTED\n\n"
"1. **What I verified** — " + "the call site is right, " * 60 + "\n\n"
"2. **What I could not verify** — " + "nothing much, " * 20 + "\n\n"
"3. **Why**\nIt widens the repository trait's return type."
)

kept = review_reason(verdict)

assert "widens the repository trait" in kept, "the objection must survive"
assert len(kept) <= 1300
# And the old behaviour must not come back: the head is a summary of a
# diff the reader already has.
assert not kept.startswith("REJECTED\n\n1. **What I verified**")


def test_a_short_verdict_is_kept_whole() -> None:
from agent_harness.executor import review_reason

assert review_reason("REJECTED\nToo narrow.") == "REJECTED\nToo narrow."
Loading