Skip to content

Retire the automated review loop #818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Neonforge98 merged 7 commits into from
Aug 18, 2026
+126 −5,259
Merged

Retire the automated review loop #818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
06319c5
refactor(orchestrator): retire the automated review loop backend
Neonforge98 Aug 15, 2026
25e949b
refactor(work-items): drop review panel actions and orchestrator hook
Neonforge98 Aug 15, 2026
eec1646
refactor(work-items): remove review settings surface
Neonforge98 Aug 15, 2026
d01c18f
chore(i18n): drop review-loop locale keys
Neonforge98 Aug 15, 2026
508078d
refactor(work-items): drop reviewer property field
Neonforge98 Aug 15, 2026
26e8f0e
refactor(work-items): remove orphaned agent settings panel
Neonforge98 Aug 15, 2026
5dee00f
chore(clippy): allow eight-arg wake-window merge helper
Neonforge98 Aug 18, 2026
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
187 changes: 16 additions & 171 deletions src-tauri/crates/agent-core/src/foundation/tool_infra/project/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,109 +68,6 @@ pub(crate) fn build_agent_prompt(
parts.join("\n")
}

/// Build a review task prompt (Rust port of the frontend's
/// `buildReviewTaskPrompt` — the backend launches review sessions now).
pub(crate) fn build_review_prompt(
short_id: &str,
frontmatter: &WorkItemFrontmatter,
body: &str,
) -> String {
let mut parts = Vec::new();
parts.push(format!(
"Review the code changes for work item: {}",
short_id
));
parts.push(format!("\n## Work Item Title\n{}", frontmatter.title));
if !body.is_empty() {
parts.push(format!("\n## Work Item Description\n{}", body));
}
if !frontmatter.todos.is_empty() {
parts.push("\n## Acceptance Criteria / Todos".to_string());
for todo in &frontmatter.todos {
let check = if todo.status == super::helpers::TODO_STATUS_COMPLETED {
"[x]"
} else {
"[ ]"
};
parts.push(format!(" {} {}", check, todo.content));
}
}

let branch = frontmatter
.proof_of_work
.as_ref()
.and_then(|pow| pow.branch.clone());
parts.push("\n## Branch Information".to_string());
if let Some(ref branch_name) = branch {
parts.push(format!("- Work item branch: `{}`", branch_name));
}
parts.push("- Base branch: `main`".to_string());
parts.push(format!(
"- Run: `git diff main..{}` to see all changes",
branch.as_deref().unwrap_or("HEAD")
));

if let Some(feedback) = frontmatter
.proof_of_work
.as_ref()
.and_then(|pow| pow.review_feedback.as_ref())
.filter(|fb| !matches!(fb.outcome, ReviewOutcome::Approved))
{
parts.push("\n## Previous Review Feedback (from the last review round)".to_string());
parts.push(format!("Outcome: {:?}", feedback.outcome));
parts.push(format!("Summary: {}", feedback.summary));
let round = frontmatter
.proof_of_work
.as_ref()
.map(|pow| pow.review_history.len())
.unwrap_or(0)
+ 1;
parts.push(format!(
"\nThis is review round {}. Check if the above issues from the previous round \
have been addressed in the current diff.",
round
));
}

parts.join("\n")
}

/// Append review feedback the coding agent must address (fix rounds).
pub(crate) fn append_fix_feedback(prompt: &mut String, frontmatter: &WorkItemFrontmatter) {
let Some(feedback) = frontmatter
.proof_of_work
.as_ref()
.and_then(|pow| pow.review_feedback.as_ref())
.filter(|fb| !matches!(fb.outcome, ReviewOutcome::Approved))
else {
return;
};

prompt.push_str("\n\n## Review Feedback To Address\n");
prompt.push_str(&format!("Outcome: {:?}\n", feedback.outcome));
prompt.push_str(&format!("Summary: {}\n", feedback.summary));
if !feedback.comments.is_empty() {
prompt.push_str("Address all of the following items before finishing:\n");
for (idx, comment) in feedback.comments.iter().enumerate() {
let loc = match (&comment.file_path, comment.line) {
(Some(path), Some(line)) => format!("{}:{} — ", path, line),
(Some(path), None) => format!("{} — ", path),
_ => String::new(),
};
prompt.push_str(&format!(
" {}. [{:?}] {}{}\n",
idx + 1,
comment.severity,
loc,
comment.message
));
}
}
prompt.push_str(
"\nFix the issues above explicitly and verify they are resolved before you finish.\n",
);
}

fn parse_agent_defs_for_execution(
content: &str,
path: &std::path::Path,
Expand Down Expand Up @@ -756,90 +653,38 @@ pub async fn start_work_item_session_with_reason(
/// Which post-transition session the orchestrator needs launched.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PhaseLaunch {
/// State machine moved to Review and already queued a pending review
/// linked session — launch the reviewer.
Review,
/// Review requested changes; state machine moved back to Coding and
/// queued a pending coding linked session — launch the fix round.
Fix,
/// Coding failed and auto-retry is on; phase is already back at Coding —
/// relaunch the owner agent.
Retry,
}

/// Launch the session demanded by an orchestrator transition
/// (LaunchReview / LaunchFix / RetryAgent). Unlike [`start_work_item`] this
/// does NOT run `snapshot_config` or phase validation: the state machine has
/// already performed the transition; we only materialize the session.
///
/// Review reuses the work item's account; the review config may override
/// account/model.
/// Launch the session demanded by an orchestrator transition (RetryAgent).
/// Unlike [`start_work_item`] this does NOT run `snapshot_config` or phase
/// validation: the state machine has already performed the transition; we
/// only materialize the session.
pub async fn launch_phase_session(
project_slug: &str,
short_id: &str,
app: &tauri::AppHandle,
phase: PhaseLaunch,
) -> Result<String, String> {
let (review_account, review_model) = if phase == PhaseLaunch::Review {
let ctx_data = run_blocking("read_review_config", {
let slug = project_slug.to_string();
let sid = short_id.to_string();
move || io::read_work_item(&slug, &sid)
})
.await?;
let review_config = ctx_data
.frontmatter
.orchestrator_config
.as_ref()
.and_then(|c| c.effective_review_config());
(
review_config.as_ref().and_then(|rc| rc.account_id.clone()),
review_config.as_ref().and_then(|rc| rc.model_id.clone()),
)
} else {
(None, None)
};

let ctx = resolve_launch_context(
project_slug,
short_id,
review_account.as_deref(),
review_model.as_deref(),
None,
)
.await?;
let PhaseLaunch::Retry = phase;
let ctx = resolve_launch_context(project_slug, short_id, None, None, None).await?;

let (agent_role, mut prompt) = match phase {
PhaseLaunch::Review => (
"review".to_string(),
build_review_prompt(short_id, &ctx.data.frontmatter, &ctx.data.body),
),
PhaseLaunch::Fix | PhaseLaunch::Retry => {
let mut prompt = if ctx.agent_def.is_some() {
build_agent_prompt(short_id, &ctx.data.frontmatter, &ctx.data.body)
} else {
build_project_prompt(short_id, &ctx.data.frontmatter, &ctx.data.body)
};
if phase == PhaseLaunch::Fix {
append_fix_feedback(&mut prompt, &ctx.data.frontmatter);
}
let role = ctx
.agent_def
.as_ref()
.map(|d| d.name.clone())
.unwrap_or_else(|| "sde".to_string());
(role, prompt)
}
let mut prompt = if ctx.agent_def.is_some() {
build_agent_prompt(short_id, &ctx.data.frontmatter, &ctx.data.body)
} else {
build_project_prompt(short_id, &ctx.data.frontmatter, &ctx.data.body)
};
let agent_role = ctx
.agent_def
.as_ref()
.map(|d| d.name.clone())
.unwrap_or_else(|| "sde".to_string());

append_workspace_section(&mut prompt, &ctx);

// Review sessions run the plain SDE harness (no custom agent definition)
// so the reviewer judges with fresh eyes.
let agent_definition_id = match phase {
PhaseLaunch::Review => None,
_ => ctx.agent_def_id.clone(),
};
let agent_definition_id = ctx.agent_def_id.clone();

let session_id = crate::session::launch::launch_agent_session(
app,
Expand Down
176 changes: 0 additions & 176 deletions src-tauri/crates/agent-core/src/orchestrator_notify/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,182 +7,6 @@
//! of the session log and shapes it into a typed payload — the dispatch
//! site lives in `orchestrator_notify::mod`.

pub(super) fn extract_review_feedback(
session_id: &str,
) -> Option<core_types::workflow::ReviewFeedback> {
use core_types::workflow::{ReviewFeedback, ReviewOutcome};

let messages = crate::session::persistence::load_messages(session_id).ok()?;

let last_assistant = messages
.iter()
.rev()
.find(|msg| msg.role == "assistant" && !msg.content.is_empty())?;

let content = &last_assistant.content;

if let Some(feedback) = parse_structured_review_block(content, session_id) {
return Some(feedback);
}

tracing::debug!(
"[review] No structured block found for session {}, falling back to keyword heuristics",
session_id
);
let content_lower = content.to_lowercase();
let outcome = if content_lower.contains("approved")
&& !content_lower.contains("not approved")
&& !content_lower.contains("changes needed")
&& !content_lower.contains("changes requested")
{
ReviewOutcome::Approved
} else {
ReviewOutcome::ChangesRequested
};

let summary = extract_first_sentence(content);

Some(ReviewFeedback {
outcome,
summary,
comments: Vec::new(),
session_id: session_id.to_string(),
reviewed_at: chrono::Utc::now().to_rfc3339(),
resolved_from_previous: Vec::new(),
})
}

pub(crate) fn parse_structured_review_block(
content: &str,
session_id: &str,
) -> Option<core_types::workflow::ReviewFeedback> {
use core_types::workflow::{ReviewComment, ReviewFeedback, ReviewOutcome};

let start_marker = "---REVIEW_START---";
let end_marker = "---REVIEW_END---";

let start_idx = content.find(start_marker)?;
let block_start = start_idx + start_marker.len();
let end_idx = content[block_start..].find(end_marker)?;
let block = &content[block_start..block_start + end_idx];

let mut outcome: Option<ReviewOutcome> = None;
let mut summary = String::new();
let mut comments: Vec<ReviewComment> = Vec::new();
let mut in_issues = false;

for line in block.lines() {
let trimmed = line.trim();
if trimmed.is_empty() {
continue;
}

if let Some(rest) = trimmed.strip_prefix("VERDICT:") {
let verdict = rest.trim().to_uppercase();
outcome = Some(match verdict.as_str() {
"APPROVED" => ReviewOutcome::Approved,
"CHANGES_REQUESTED" => ReviewOutcome::ChangesRequested,
_ => ReviewOutcome::ChangesRequested,
});
in_issues = false;
} else if let Some(rest) = trimmed.strip_prefix("SUMMARY:") {
summary = rest.trim().to_string();
in_issues = false;
} else if trimmed == "ISSUES:" {
in_issues = true;
} else if in_issues && trimmed.starts_with("- [") {
if let Some(comment) = parse_issue_line(trimmed) {
comments.push(comment);
}
}
}

let outcome = outcome?;

Some(ReviewFeedback {
outcome,
summary,
comments,
session_id: session_id.to_string(),
reviewed_at: chrono::Utc::now().to_rfc3339(),
resolved_from_previous: Vec::new(),
})
}

pub(crate) fn parse_issue_line(line: &str) -> Option<core_types::workflow::ReviewComment> {
use core_types::workflow::{ReviewComment, ReviewCommentSeverity};

let rest = line.strip_prefix("- ")?;

let (severity, after_tag) = if let Some(r) = rest.strip_prefix("[ERROR]") {
(ReviewCommentSeverity::Error, r)
} else if let Some(r) = rest.strip_prefix("[WARNING]") {
(ReviewCommentSeverity::Warning, r)
} else if let Some(r) = rest.strip_prefix("[SUGGESTION]") {
(ReviewCommentSeverity::Suggestion, r)
} else {
let r = rest.strip_prefix("[PRAISE]")?;
(ReviewCommentSeverity::Praise, r)
};

let after_tag = after_tag.trim();

let (file_path, line_num, message) =
if let Some(dash_pos) = after_tag.find(" — ").or_else(|| after_tag.find(" - ")) {
let location = &after_tag[..dash_pos].trim();
let dash_len = if after_tag[dash_pos..].starts_with(" — ") {
" — ".len()
} else {
" - ".len()
};
let msg = after_tag[dash_pos + dash_len..].trim().to_string();

let (fp, ln) = parse_file_location(location);
(fp, ln, msg)
} else {
(None, None, after_tag.to_string())
};

Some(ReviewComment {
file_path,
line: line_num,
severity,
message,
})
}

pub(crate) fn parse_file_location(location: &str) -> (Option<String>, Option<u32>) {
if location.is_empty() {
return (None, None);
}
if let Some(colon_pos) = location.rfind(':') {
let path_part = &location[..colon_pos];
let line_part = &location[colon_pos + 1..];
if let Ok(line_num) = line_part.parse::<u32>() {
return (Some(path_part.to_string()), Some(line_num));
}
}
(Some(location.to_string()), None)
}

pub(crate) fn extract_first_sentence(content: &str) -> String {
let trimmed = content.trim();
for (idx, ch) in trimmed.char_indices() {
if (ch == '.' || ch == '!' || ch == '\n') && idx > 10 {
let sentence = trimmed[..=idx].trim();
if sentence.len() <= 500 {
return sentence.to_string();
}
}
}
let truncated: String = crate::utils::safe_truncate_chars_to_string(&trimmed, 300);
if truncated.len() < trimmed.len() {
format!("{}…", truncated.trim_end())
} else {
truncated
}
}

/// Proof-of-work facts gathered from git, decoupled from the frontmatter
/// mutation so the subprocess I/O can run OUTSIDE the work item's
/// `BEGIN IMMEDIATE` transaction. Running git inside the transaction
Expand Down
Loading
Loading