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
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,8 @@ def _run_followup_review_once(
f"{note} The reviewed head is no longer an ancestor of HEAD; review the current branch diff "
f"against {base} and focus on whether the fixes address that source round's findings."
)
if review_instructions := _review_instructions(state, {}):
note = f"{note}\n\n{review_instructions}"
prompt = build_followup_prompt(
since_head=since_head,
head=head,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
public_task_name,
read_jsonl,
round_needs_caller_grade,
terminal_review_command,
unique_round_state_dirs,
)

Expand Down Expand Up @@ -605,8 +604,6 @@ def _round_terminal_command(round_record: dict[str, object]) -> str | None:
if status and status != "completed":
return None
command = str(run.get("terminal_command") or "").strip().lower()
if not command:
command = terminal_review_command(str(run.get("reviewer_output") or ""))
if command not in DECISION_COMMANDS:
return None
commands.append(command)
Expand Down Expand Up @@ -726,13 +723,24 @@ def _orchestrator_action(
}
else:
action = {
"cmd": _review_command(public_id, extra=("--decision", "clean")),
"alt": _review_command(public_id, extra=("--decision", "findings")),
"choices": {
decision: _review_command(
public_id, extra=("--decision", decision)
)
for decision in ("clean", "findings")
},
"note": "Classify the reviewer output, then record clean or findings.",
}
elif stage == "fix-pending":
note = "Commit/amend valid fixes, then rerun this command."
if str(state.get("review_brief") or "").strip():
note += (
" If a finding conflicts with the frozen contract, rerun this review "
"id with --contract-conflict <dimension> instead."
)
action = {
"cmd": _review_command(public_id),
"note": "Commit/amend valid fixes, then rerun this command.",
"note": note,
}
elif stage in {"review-green", "local-green-handoff"}:
summary = review_ladder_summary(state, current_head=current_head)
Expand Down
2 changes: 2 additions & 0 deletions plugins/review-suite/tests/test_review_orchestrator_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -1430,6 +1430,7 @@ def test_runner_runs_real_followup_once_from_followup_pending(
fixed["identity"]["requested_base"] = "main"
fixed["identity"]["base_upstream"] = "origin/main"
fixed["identity"]["base_ref_stale"] = True
fixed["review_brief"] = "# Goal\n\nStay scoped."

result = orchestrator_runner.run_one_expensive_step(
fixed, state_dir=tmp_path / "state"
Expand Down Expand Up @@ -1468,6 +1469,7 @@ def test_runner_runs_real_followup_once_from_followup_pending(
assert "Source review round phase_review-round-1" in str(
followup_calls[0]["prompt"]
)
_assert_review_brief_instructions(followup_calls[0]["prompt"])


def test_runner_runs_rewritten_followup_against_branch_scope(
Expand Down
28 changes: 22 additions & 6 deletions plugins/review-suite/tests/test_review_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,16 +622,21 @@ def test_review_state_status_surfaces_orchestrator_progress(
assert emitted[0]["progress"] == "review 1/2 broad-discovery"
assert "recommendation" not in emitted[0]
assert "reason" not in emitted[0]
assert "--id rvw_progress --decision clean" in str(emitted[0]["Action"]["cmd"])
assert "--id rvw_progress --decision findings" in str(emitted[0]["Action"]["alt"])
action = emitted[0]["Action"]
assert set(action) == {"choices", "note", "restart"}
assert action["note"] == (
"Classify the reviewer output, then record clean or findings."
)
assert "--id rvw_progress --decision clean" in str(action["choices"]["clean"])
assert "--id rvw_progress --decision findings" in str(action["choices"]["findings"])
assert "--id rvw_progress --restart-mode deep --reason REASON" in str(
emitted[0]["Action"]["restart"]["cmd"]
)
assert emitted[0]["Action"]["restart"]["mode"] == "deep"
assert "--state-dir" not in str(emitted[0]["Action"]["cmd"])
assert "--state-dir" not in str(emitted[0]["Action"]["alt"])
assert "--state-dir" not in str(action["choices"]["clean"])
assert "--state-dir" not in str(action["choices"]["findings"])
assert "--state-dir" not in str(emitted[0]["Action"]["restart"]["cmd"])
assert str(state_dir.resolve(strict=False)) not in str(emitted[0]["Action"]["cmd"])
assert str(state_dir.resolve(strict=False)) not in str(action["choices"]["clean"])
assert "review_t1.py" not in str(emitted[0]["Action"])


Expand Down Expand Up @@ -773,7 +778,8 @@ def test_review_state_status_uses_bare_id_for_structured_verdict(
{
"slot": "alpha",
"review_status": "completed",
"reviewer_output": "No findings.\n\nReview result: clean",
"reviewer_output": "Review result: findings",
"terminal_command": "clean",
"grade_blocked": False,
}
],
Expand Down Expand Up @@ -818,6 +824,16 @@ def test_review_state_status_uses_bare_id_for_structured_verdict(
assert "--id rvw_progress --decision findings" in str(
action["override"]["findings"]
)
briefed_fix = review_state._orchestrator_action(
{"stage": "fix-pending", "review_brief": "Frozen contract"},
"rvw_progress",
state_dir=state_dir,
)
briefless_fix = review_state._orchestrator_action(
{"stage": "fix-pending"}, "rvw_progress", state_dir=state_dir
)
assert "--contract-conflict <dimension>" in str(briefed_fix["note"])
assert briefless_fix["note"] == "Commit/amend valid fixes, then rerun this command."


def test_review_state_status_surfaces_grade_before_structured_verdict(
Expand Down