feat: enhance context gathering in auto-review engine and agent runner

2026-07-20 17:32:31 +07:00
parent 289c58ef36
commit dcc8c3ee42
4 changed files with 183 additions and 105 deletions
@@ -1,12 +1,12 @@
-//! Auto-review engine — after edits, spawns a subagent that reviews AND
-//! automatically fixes issues using tool access (edit/write/grep).
+//! Auto-review engine — after edits, reviews AND auto-fixes issues using
+//! tools + LLM, all synchronously in a background thread (no tokio runtime
+//! needed).
 //!
 //! Flow: after an agent turn with edits completes:
 //!   1. Emit `WorkflowAgentUpdate(Running)` → visible in workflow sidebar
 //!   2. Run `git diff` to get the changed files
-//!   3. Spawn the subagent engine with Write-tier tools + directive to
-//!      review & fix
-//!   4. The subagent finds issues and applies fixes using edit/write tools
+//!   3. Call LLM with the diff to identify issues and suggested fixes
+//!   4. Apply fixes using sync tools (edit/write)
 //!   5. Results stream as `TurnEvent::SystemNote` events
 //!   6. Emit `WorkflowAgentUpdate(Completed)` when done

@@ -17,26 +17,19 @@ use std::sync::{Arc, Mutex};

 use tracing::{debug, info, instrument, warn};

-use crate::subagent::context::SubagentContext;
-use crate::subagent::division::AccessTier;
-use crate::subagent::spawn::spawn_subagent;
-use crate::tools::ToolCtx;
+use crate::llm::provider::LlmClient;
+use crate::subagent::division::{tools_for, AccessTier};
+use crate::tools::{Tool, ToolCtx};
 use crate::{AgentStatus, TurnEvent};
+use zesdex_domain::core::ChatMessage;

 const REVIEW_AGENT_ID: &str = "auto-review";

-/// Spawn a review subagent that reviews changes and auto-fixes issues.
+/// Spawn a background thread that reviews changes and auto-fixes issues.
 ///
-/// The subagent runs inline on the current background thread (no extra
-/// thread spawn) with its own tokio runtime and Write-tier tool access
-/// (edit, write, grep, read, glob). It receives the git diff as context
-/// and is directed to:
-///   1. Read changed files
-///   2. Check for typos, missing imports, syntax errors, logic bugs
-///   3. Fix any issues found using edit/write tools
-///   4. Report what was fixed
-///
-/// All findings stream as TurnEvent events consumed by the TUI event loop.
+/// Everything runs synchronously on the background thread — no tokio
+/// runtime is created, avoiding the nested-runtime panic from
+/// reqwest::blocking inside block_on in tokio >= 1.38.
 #[instrument(skip(turn_events))]
 pub fn spawn_background_review(
    workspace_roots: Vec<PathBuf>,
@@ -98,7 +91,32 @@ pub fn spawn_background_review(
            }
        };

-        // Truncate very large diffs for the prompt
+        push_event(
+            &turn_events,
+            TurnEvent::SystemNote {
+                kind: "review".into(),
+                message: "🔍 Auto-review: examining and fixing issues...".into(),
+            },
+        );
+
+        // 2. Build tool context + load write-tier tools
+        let tool_ctx = ToolCtx::builder()
+            .session_dir(root.join(".zesdex").join("sessions").join("auto-review"))
+            .workspaces(workspace_roots.clone())
+            .turn_events(turn_events.clone())
+            .build();
+
+        let tools = tools_for(&AccessTier::Write);
+
+        // 3. Resolve LLM credentials
+        let base_url = api_base.unwrap_or_else(|| {
+            std::env::var("OPENAI_API_BASE")
+                .unwrap_or_else(|_| "https://opencode.ai/zen/v1".to_string())
+        });
+
+        let client = LlmClient::new(api_key, model, Some(base_url));
+
+        // 4. Truncate diff if needed
        const MAX_DIFF_CHARS: usize = 5000;
        let truncated_diff = if diff.len() > MAX_DIFF_CHARS {
            push_event(
@@ -121,78 +139,50 @@ pub fn spawn_background_review(
            diff.to_string()
        };

-        // 2. Build directive: review AND fix issues using tools
-        let directive = format!(
-            "You are an auto-review subagent. Complete the following:\n\n\
-             1. Review this git diff for:\n\
+        // 5. Call LLM to review the diff and suggest fixes.
+        //    No tokio runtime needed — LlmClient uses reqwest::blocking
+        //    internally, which is fine on a plain thread.
+        let system_msg = ChatMessage::system(
+            "You are an auto-review subagent. Your ONLY job:\n\
+             1. Review the git diff below for:\n\
                - Typos and spelling errors\n\
                - Missing imports or undefined references\n\
                - Syntax errors or type mismatches\n\
                - Logic bugs or off-by-one errors\n\
                - Missing error handling\n\
                - Security issues\n\n\
-             2. FIX any issues you find using the available tools:\n\
-                - `read` to check file contents\n\
-                - `edit` to fix specific text blocks\n\
-                - `write` to replace files if needed\n\
-                - `grep` to find related patterns\n\n\
-             3. Be conservative: only fix CLEAR, CONFIRMED issues. \
-                Don't change logic, style, or formatting.\n\
-             4. Report what you fixed at the end.\n\n\
-             Git diff of changes:\n\n```diff\n{truncated_diff}\n```"
+             2. For each issue found, output a command to fix it.\n\n\
+             Available commands:\n\
+             - `edit <file>` then provide the old text and new text\n\
+             - `write <file>` then provide the new content\n\n\
+             Output format:\n\
+             If no issues: NO_ISSUES_FOUND\n\n\
+             If issues found:\n\
+             ---\n\
+             FILE: <path>\n\
+             ISSUE: <description>\n\
+             SEVERITY: HIGH|MEDIUM|LOW\n\
+             OLD: <exact text to replace>\n\
+             NEW: <replacement text>\n\
+             ---".to_string(),
        );

-        // 3. Emit progress note
-        push_event(
-            &turn_events,
-            TurnEvent::SystemNote {
-                kind: "review".into(),
-                message: "🔍 Auto-review: examining and fixing issues...".into(),
-            },
-        );
+        let user_msg = ChatMessage::user(format!(
+            "Review and fix this git diff:\n\n```diff\n{truncated_diff}\n```"
+        ));

-        // 4. Build minimal ToolCtx
-        let tool_ctx = ToolCtx::builder()
-            .session_dir(root.join(".zesdex").join("sessions").join("auto-review"))
-            .workspaces(workspace_roots.clone())
-            .turn_events(turn_events.clone())
-            .build();
+        // This is a sync call — no tokio runtime required on this thread.
+        let response = run_llm_review(&client, &[system_msg, user_msg]);

-        // 5. Determine base URL
-        let base_url = api_base.unwrap_or_else(|| {
-            std::env::var("OPENAI_API_BASE")
-                .unwrap_or_else(|_| "https://opencode.ai/zen/v1".to_string())
-        });
-
-        // 6. Build SubagentContext
-        let subagent_ctx = SubagentContext::new(
-            directive,
-            tool_ctx.clone(),
-            "write".to_string(),
-            base_url,
-            api_key,
-            model,
-        );
-
-        // 7. Spawn the subagent via spawn_subagent (creates its own OS thread
-        //    + tokio runtime internally, avoiding nested runtime panics).
-        let handle = spawn_subagent(
-            subagent_ctx,
-            "Auto-review and fix issues in the changed files".to_string(),
-            AccessTier::Write,
-            tool_ctx,
-        );
-
-        // 8. Report results
-        let report = match handle.join() {
-            Ok(Ok(r)) => r,
-            Ok(Err(e)) => {
-                warn!(error = %e, "auto-review subagent failed");
+        let response_text = match response {
+            Ok(text) => text,
+            Err(e) => {
+                warn!(error = %e, "auto-review LLM call failed");
                push_event(
                    &turn_events,
                    TurnEvent::SystemNote {
                        kind: "review".into(),
-                        message: format!("⚠️ Auto-review encountered an error: {e}"),
+                        message: format!("⚠️ Auto-review failed: {e}"),
                    },
                );
                push_event(
@@ -205,29 +195,10 @@ pub fn spawn_background_review(
                );
                return;
            }
-            Err(e) => {
-                warn!(error = ?e, "auto-review subagent panicked");
-                push_event(
-                    &turn_events,
-                    TurnEvent::SystemNote {
-                        kind: "review".into(),
-                        message: "⚠️ Auto-review agent panicked.".into(),
-                    },
-                );
-                push_event(
-                    &turn_events,
-                    TurnEvent::WorkflowAgentUpdate {
-                        agent_id,
-                        agent_name,
-                        status: AgentStatus::Failed("panicked".to_string()),
-                    },
-                );
-                return;
-            }
        };

-        let trimmed = report.trim();
-        if trimmed.is_empty() || trimmed.to_lowercase().contains("no issues") {
+        // 6. Parse and apply fixes
+        if response_text.trim() == "NO_ISSUES_FOUND" || response_text.trim().is_empty() {
            push_event(
                &turn_events,
                TurnEvent::SystemNote {
@@ -237,15 +208,19 @@ pub fn spawn_background_review(
            );
            info!("auto-review: no issues found");
        } else {
+            // Try to apply structured fixes
+            let fix_count = apply_fixes_from_response(&response_text, &tools, &tool_ctx);
+
            push_event(
                &turn_events,
                TurnEvent::SystemNote {
                    kind: "review_finding".into(),
-                    message: format!("📋 Auto-review complete:\n{}", trimmed),
+                    message: format!("📋 Auto-review complete ({} fix(es) applied).\n{}", fix_count, response_text.trim()),
                },
            );
-            info!("auto-review: completed with findings");
+            info!(fix_count, "auto-review: completed with fixes");
        }
+
        push_event(
            &turn_events,
            TurnEvent::WorkflowAgentUpdate {
@@ -257,6 +232,90 @@ pub fn spawn_background_review(
    });
 }

+/// Run the LLM review call synchronously using reqwest::blocking.
+fn run_llm_review(client: &LlmClient, messages: &[ChatMessage]) -> Result<String, String> {
+    // Direct LLM call — no tokio, no tool calls, just Q&A.
+    match client.chat_with_tools_non_streaming(messages, None, Some(1024), Some(0.3), None) {
+        Ok((msg, _)) => Ok(msg.content.unwrap_or_default()),
+        Err(e) => Err(e.to_string()),
+    }
+}
+
+/// Parse the LLM response for structured fix commands and apply them.
+fn apply_fixes_from_response(
+    response: &str,
+    tools: &[Box<dyn Tool>],
+    tool_ctx: &ToolCtx,
+) -> usize {
+    let mut fix_count = 0;
+
+    // Parse structured fix blocks
+    let blocks: Vec<&str> = response.split("---").collect();
+
+    for block in &blocks {
+        let trimmed = block.trim();
+        if trimmed.is_empty() {
+            continue;
+        }
+
+        let lines: Vec<&str> = trimmed.lines().map(|l| l.trim()).collect();
+        if lines.len() < 4 {
+            continue;
+        }
+
+        // Try to extract structured fix
+        let file_path = extract_field(&lines, "FILE:").unwrap_or("");
+        let severity = extract_field(&lines, "SEVERITY:").unwrap_or("LOW");
+        let old_text = extract_field(&lines, "OLD:").unwrap_or("");
+        let new_text = extract_field(&lines, "NEW:").unwrap_or("");
+
+        if file_path.is_empty() || old_text.is_empty() || new_text.is_empty() {
+            continue;
+        }
+
+        // Only auto-fix HIGH and MEDIUM severity issues
+        if severity != "HIGH" && severity != "MEDIUM" {
+            debug!(severity, file = file_path, "skipping LOW severity fix");
+            continue;
+        }
+
+        // Try to apply the fix using the edit tool
+        if let Some(edit_tool) = tools.iter().find(|t| t.name() == "edit") {
+            let args = serde_json::json!({
+                "path": file_path,
+                "old": old_text,
+                "new": new_text,
+                "reason": "auto-review fix"
+            });
+
+            match edit_tool.run(tool_ctx, &args) {
+                Ok(result) => {
+                    info!(file = file_path, "auto-review fix applied: {result}");
+                    fix_count += 1;
+                }
+                Err(e) => {
+                    debug!(file = file_path, error = %e, "auto-review fix failed");
+                }
+            }
+        }
+    }
+
+    fix_count
+}
+
+/// Extract a field value from parsed lines (e.g. "FILE: src/main.rs" → "src/main.rs").
+fn extract_field<'a>(lines: &[&'a str], prefix: &str) -> Option<&'a str> {
+    for line in lines {
+        if let Some(val) = line.strip_prefix(prefix) {
+            let trimmed = val.trim();
+            if !trimmed.is_empty() {
+                return Some(trimmed);
+            }
+        }
+    }
+    None
+}
+
 /// Run `git diff` to get workspace changes (both staged and unstaged).
 fn get_git_diff(workspace_root: &PathBuf) -> Result<String, String> {
    let git_dir = workspace_root.join(".git");