review: expose failed review diagnostics in raw logs

Review failures were stored as review-level result descriptions,
while the patchset page mostly surfaced patchset.failed_reason.
When the review worker failed before producing a final JSON payload,
the raw log page could also be empty because no interaction logs
had been persisted.

Keep patchset details concise and route failure diagnosis through
the existing raw-log link. Make the raw log page show the final
failure reason above the interaction log, then fall back to saved
raw output or input context when no structured interaction log was recorded.

Capture partial stdout, stderr, and AI turn summaries while the
review tool runs, keep those logs if completion later stores no
log payload, and store process output with explicit process-log
metadata so it can be rendered separately from AI interactions.

Signed-off-by: Muchun Song <songmuchun@bytedance.com>

Author: songmuchun

src/db.rs

@@ -762,7 +762,7 @@ impl Database {
     ) -> Result<()> {
         self.conn
             .execute(
-                "UPDATE reviews SET status = ?, result_description = ?, summary = ?, interaction_id = ?, inline_review = ?, logs = ? WHERE id = ?",
+                "UPDATE reviews SET status = ?, result_description = ?, summary = ?, interaction_id = ?, inline_review = ?, logs = COALESCE(?, logs) WHERE id = ?",
                 libsql::params![status, result, summary, interaction_id, inline_review, logs, review_id],
             )
             .await?;

src/reviewer.rs

@@ -35,9 +35,12 @@ use std::process::Stdio;
 use std::sync::Arc;
 use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
 use tokio::process::Command;
-use tokio::sync::Semaphore;
+use tokio::sync::{Mutex, Semaphore};
 use tracing::{error, info, warn};
 
+const REVIEW_LOG_MAX_ENTRIES: usize = 2000;
+const REVIEW_LOG_STRING_LIMIT: usize = 12_000;
+
 #[derive(Clone)]
 struct ReviewContext {
     semaphore: Arc<Semaphore>,
@@ -1473,6 +1476,72 @@ impl Reviewer {
     }
 }
 
+fn truncate_review_log_string(s: &str) -> String {
+    let redacted = redact_secret(s);
+    if redacted.chars().count() <= REVIEW_LOG_STRING_LIMIT {
+        return redacted;
+    }
+
+    let mut truncated: String = redacted.chars().take(REVIEW_LOG_STRING_LIMIT).collect();
+    truncated.push_str("\n...[truncated]");
+    truncated
+}
+
+fn truncate_review_log_value(value: &mut Value) {
+    match value {
+        Value::String(s) => {
+            *s = truncate_review_log_string(s);
+        }
+        Value::Array(values) => {
+            for value in values {
+                truncate_review_log_value(value);
+            }
+        }
+        Value::Object(map) => {
+            for value in map.values_mut() {
+                truncate_review_log_value(value);
+            }
+        }
+        _ => {}
+    }
+}
+
+async fn push_review_log_entry(log: &Arc<Mutex<Vec<Value>>>, mut entry: Value) {
+    truncate_review_log_value(&mut entry);
+
+    let mut entries = log.lock().await;
+    if entries.len() < REVIEW_LOG_MAX_ENTRIES {
+        entries.push(entry);
+    } else if entries.len() == REVIEW_LOG_MAX_ENTRIES {
+        entries.push(json!({
+            "role": "system",
+            "content": "Additional review log entries were omitted."
+        }));
+    }
+}
+
+async fn push_review_log_text(log: &Arc<Mutex<Vec<Value>>>, stream: &str, line: &str) {
+    push_review_log_entry(
+        log,
+        json!({
+            "role": "system",
+            "kind": "process_log",
+            "stream": stream,
+            "content": line
+        }),
+    )
+    .await;
+}
+
+async fn review_logs_json(log: &Arc<Mutex<Vec<Value>>>) -> Option<String> {
+    let entries = log.lock().await;
+    if entries.is_empty() {
+        None
+    } else {
+        serde_json::to_string_pretty(&*entries).ok()
+    }
+}
+
 #[allow(clippy::too_many_arguments)]
 async fn run_review_tool(
     patchset_id: i64,
@@ -1575,12 +1644,15 @@ async fn run_review_tool(
     cmd.kill_on_drop(true);
 
     let mut child = cmd.spawn()?;
+    let captured_log: Arc<Mutex<Vec<Value>>> = Arc::new(Mutex::new(Vec::new()));
 
-    if let Some(stderr) = child.stderr.take() {
-        tokio::spawn(async move {
+    let stderr_task = if let Some(stderr) = child.stderr.take() {
+        let captured_log = captured_log.clone();
+        Some(tokio::spawn(async move {
             let reader = BufReader::new(stderr);
             let mut lines = reader.lines();
             while let Ok(Some(line)) = lines.next_line().await {
+                push_review_log_text(&captured_log, "stderr", &line).await;
                 if line.contains(" ERROR ")
                     || line.starts_with("Error:")
                     || line.contains("panicked")
@@ -1592,8 +1664,10 @@ async fn run_review_tool(
                     info!("[review-bin] {}", line);
                 }
             }
-        });
-    }
+        }))
+    } else {
+        None
+    };
 
     let mut stdin = child
         .stdin
@@ -1665,6 +1739,26 @@ async fn run_review_tool(
                                         serde_json::from_value::<AiRequest>(payload_val.clone())
                                 {
                                     turn_count += 1;
+                                    let mut request_entry = req
+                                        .messages
+                                        .last()
+                                        .and_then(|m| serde_json::to_value(m).ok())
+                                        .unwrap_or_else(|| {
+                                            json!({
+                                                "role": "user",
+                                                "content": "AI request contained no messages"
+                                            })
+                                        });
+                                    if let Some(obj) = request_entry.as_object_mut() {
+                                        obj.insert(
+                                            "context_tag".to_string(),
+                                            Value::String(
+                                                req.context_tag.clone().unwrap_or_default(),
+                                            ),
+                                        );
+                                        obj.insert("turn".to_string(), json!(turn_count));
+                                    }
+                                    push_review_log_entry(&captured_log, request_entry).await;
                                     if settings.ai.log_turns {
                                         let n_msgs = req.messages.len();
                                         let last = req.messages.last();
@@ -1726,6 +1820,21 @@ async fn run_review_tool(
 
                                     let reply = match resp_payload {
                                         Ok(p) => {
+                                            let mut response_entry = serde_json::to_value(&p)
+                                                .unwrap_or_else(|_| {
+                                                    json!({
+                                                        "content": "Failed to serialize AI response"
+                                                    })
+                                                });
+                                            if let Some(obj) = response_entry.as_object_mut() {
+                                                obj.insert(
+                                                    "role".to_string(),
+                                                    Value::String("assistant".to_string()),
+                                                );
+                                                obj.insert("turn".to_string(), json!(turn_count));
+                                            }
+                                            push_review_log_entry(&captured_log, response_entry)
+                                                .await;
                                             if let Some(usage) = &p.usage {
                                                 let cached = usage.cached_tokens.unwrap_or(0);
                                                 let uncached_input = usage.prompt_tokens.saturating_sub(cached);
@@ -1791,6 +1900,14 @@ async fn run_review_tool(
                                         Err(e) => {
                                             let message = e.to_string();
                                             let class = classify_ai_error(&e);
+                                            push_review_log_entry(
+                                                &captured_log,
+                                                json!({
+                                                    "role": "system",
+                                                    "content": format!("AI provider error: {message}")
+                                                }),
+                                            )
+                                            .await;
                                             let payload = RemoteAiErrorPayload::new(message, class);
                                             json!({ "type": "error", "payload": payload })
                                         }
@@ -1809,6 +1926,8 @@ async fn run_review_tool(
                                 if json_msg.get("patchset_id").is_some() {
                                     final_result = Some(json_msg);
                                     break;
+                                } else {
+                                    push_review_log_text(&captured_log, "stdout", &line).await;
                                 }
                             }
                         }
@@ -1817,10 +1936,13 @@ async fn run_review_tool(
                         if json_msg.get("patchset_id").is_some() {
                             final_result = Some(json_msg);
                             break;
+                        } else {
+                            push_review_log_text(&captured_log, "stdout", &line).await;
                         }
                     }
                 } else {
                     // Non-JSON line. Log it.
+                    push_review_log_text(&captured_log, "stdout", &line).await;
                     warn!("Review tool stdout: {}", line);
                 }
             }
@@ -1838,9 +1960,19 @@ async fn run_review_tool(
     // Interaction finished (Success or Error inside interaction)
     drop(stdin); // Close stdin to signal EOF/finish to child if it's still running
     let _ = child.wait().await; // Reap zombie
+    if let Some(task) = stderr_task {
+        let _ = task.await;
+    }
 
     match interaction_result {
-        Ok(json) => {
+        Ok(mut json) => {
+            if json.get("history").is_none()
+                && let Some(logs) = review_logs_json(&captured_log).await
+                && let Ok(history) = serde_json::from_str::<Value>(&logs)
+                && let Some(obj) = json.as_object_mut()
+            {
+                obj.insert("history".to_string(), history);
+            }
             // Update DB with patch statuses if final_result available
             if let Some(patches) = json["patches"].as_array() {
                 for p in patches {
@@ -1887,6 +2019,16 @@ async fn run_review_tool(
             Ok(json)
         }
         Err(e) => {
+            if let Some(logs) = review_logs_json(&captured_log).await
+                && let Err(log_err) = db
+                    .update_review_status(review_id, ReviewStatus::InReview.as_str(), Some(&logs))
+                    .await
+            {
+                warn!(
+                    "Failed to persist partial review log for review {}: {}",
+                    review_id, log_err
+                );
+            }
             // Check if it's the specific active time exceeded error we throw in the loop
             if e.to_string()
                 .contains("Review tool timed out (active time exceeded)")