Skip to main content

fix_engine/
goose_client.rs

1//! Goose headless client for AI-assisted fix generation.
2//!
3//! Shells out to `goose run` with the developer extension to apply
4//! complex migration fixes that can't be handled by pattern matching.
5
6use anyhow::{Context, Result};
7use fix_engine_core::LlmFixRequest;
8
9use crate::context::FixContext;
10use std::collections::BTreeMap;
11use std::path::PathBuf;
12use std::process::Command;
13use std::time::Duration;
14
15#[cfg(unix)]
16use std::os::unix::process::CommandExt;
17
18/// Per-file timeout for goose subprocess (seconds).
19const GOOSE_TIMEOUT_SECS: u64 = 120;
20
21/// Delay between consecutive goose calls to avoid rate limiting (seconds).
22const _GOOSE_DELAY_SECS: u64 = 2;
23
24/// Maximum retries when a goose call times out.
25const _GOOSE_MAX_RETRIES: u32 = 1;
26
27/// Result of a goose fix attempt.
28#[derive(Debug)]
29pub struct GooseFixResult {
30    pub file_path: PathBuf,
31    pub rule_id: String,
32    pub success: bool,
33    pub output: String,
34}
35
36/// Run a goose command with a timeout. Returns the combined stdout+stderr
37/// output, or an error if the process times out or fails to start.
38/// Return type from goose: (success, text_response, raw_json_output)
39/// The raw_json_output contains the full goose session including all
40/// tool calls, which is invaluable for debugging empty responses.
41fn run_goose_with_timeout(prompt: &str, max_turns: &str) -> Result<(bool, String, String)> {
42    let mut cmd = Command::new("goose");
43    cmd.args([
44        "run",
45        "--quiet",
46        "--text",
47        prompt,
48        "--with-builtin",
49        "developer",
50        "--no-session",
51        "--max-turns",
52        max_turns,
53        "--output-format",
54        "json",
55    ])
56    .stdout(std::process::Stdio::piped())
57    .stderr(std::process::Stdio::piped())
58    .stdin(std::process::Stdio::null());
59
60    // Isolate goose in its own process group so that signals sent by
61    // goose's child processes (e.g., claude-code) cannot propagate to
62    // our parent process.
63    #[cfg(unix)]
64    cmd.process_group(0);
65
66    let mut child = cmd
67        .spawn()
68        .context("Failed to execute goose. Is it installed and in PATH?")?;
69
70    let timeout = Duration::from_secs(GOOSE_TIMEOUT_SECS);
71    let start = std::time::Instant::now();
72
73    loop {
74        match child.try_wait() {
75            Ok(Some(status)) => {
76                // Process exited — stdout is JSON with full message history
77                let raw_json = child
78                    .stdout
79                    .take()
80                    .map(|mut s| {
81                        let mut buf = String::new();
82                        std::io::Read::read_to_string(&mut s, &mut buf).ok();
83                        buf
84                    })
85                    .unwrap_or_default();
86                let _stderr = child
87                    .stderr
88                    .take()
89                    .map(|mut s| {
90                        let mut buf = String::new();
91                        std::io::Read::read_to_string(&mut s, &mut buf).ok();
92                        buf
93                    })
94                    .unwrap_or_default();
95
96                // Extract the text response from the JSON output.
97                // The JSON has { "messages": [ { "role": "assistant", "content": [...] } ] }
98                // We want the last assistant message's text content.
99                let text_response = extract_text_from_goose_json(&raw_json);
100
101                return Ok((status.success(), text_response, raw_json));
102            }
103            Ok(None) => {
104                // Still running — check timeout
105                if start.elapsed() >= timeout {
106                    // Kill the entire process group (goose + any children like
107                    // claude-code) to prevent orphaned processes.
108                    #[cfg(unix)]
109                    {
110                        let pid = child.id() as i32;
111                        // SIGTERM first to allow graceful shutdown
112                        unsafe {
113                            libc::kill(-pid, libc::SIGTERM);
114                        }
115                        std::thread::sleep(Duration::from_millis(1000));
116                        // SIGKILL to ensure cleanup
117                        unsafe {
118                            libc::kill(-pid, libc::SIGKILL);
119                        }
120                    }
121                    #[cfg(not(unix))]
122                    {
123                        let _ = child.kill();
124                    }
125                    let _ = child.wait();
126                    anyhow::bail!("goose timed out after {}s", GOOSE_TIMEOUT_SECS);
127                }
128                std::thread::sleep(Duration::from_millis(500));
129            }
130            Err(e) => {
131                anyhow::bail!("Failed to wait on goose process: {}", e);
132            }
133        }
134    }
135}
136
137/// Run goose fixes for all pending LLM requests.
138/// Groups requests by file path for batch processing.
139/// If `log_dir` is provided, saves prompts and responses to JSON files.
140/// An LLM fix request with multiple incidents from the same rule merged
141/// into a single entry. This preserves the priority-based sort order
142/// (hierarchy rules first) rather than re-sorting by rule_id.
143#[derive(Debug)]
144struct MergedLlmFixRequest {
145    rule_id: String,
146    file_path: PathBuf,
147    /// All incident line numbers (may be a single line).
148    lines: Vec<u32>,
149    /// Rule message (shared across all incidents of the same rule).
150    message: String,
151    /// Code snippets keyed by line number.
152    code_snips: Vec<(u32, String)>,
153    /// Component family (e.g., "Modal", "Select") extracted from labels.
154    /// Used to group related rules in the batch prompt.
155    family: Option<String>,
156}
157
158/// Merge LLM fix requests by rule_id, preserving insertion order.
159///
160/// Multiple incidents from the same rule (e.g., a composition rule firing
161/// at different lines) are collapsed into a single entry with all affected
162/// lines and code snippets combined.
163fn merge_by_rule_id(requests: &[&LlmFixRequest]) -> Vec<MergedLlmFixRequest> {
164    let mut merged: Vec<MergedLlmFixRequest> = Vec::new();
165    let mut index: std::collections::HashMap<&str, usize> = std::collections::HashMap::new();
166
167    for req in requests {
168        if let Some(&idx) = index.get(req.rule_id.as_str()) {
169            merged[idx].lines.push(req.line);
170            if let Some(snip) = &req.code_snip {
171                merged[idx].code_snips.push((req.line, snip.clone()));
172            }
173        } else {
174            let idx = merged.len();
175            index.insert(&req.rule_id, idx);
176            let code_snips = req
177                .code_snip
178                .as_ref()
179                .map(|s| vec![(req.line, s.clone())])
180                .unwrap_or_default();
181            let family = req
182                .labels
183                .iter()
184                .find(|l| l.starts_with("family="))
185                .and_then(|l| l.strip_prefix("family="))
186                .map(|s| s.to_string());
187            merged.push(MergedLlmFixRequest {
188                rule_id: req.rule_id.clone(),
189                file_path: req.file_path.clone(),
190                lines: vec![req.line],
191                message: req.message.clone(),
192                code_snips,
193                family,
194            });
195        }
196    }
197
198    merged
199}
200
201/// Extract the text response from goose's JSON output format.
202///
203/// Goose's `--output-format json` returns:
204/// ```json
205/// { "messages": [ { "role": "assistant", "content": [{ "type": "text", "text": "..." }] } ] }
206/// ```
207/// We extract the text from the LAST assistant message.
208fn extract_text_from_goose_json(raw_json: &str) -> String {
209    let parsed: Result<serde_json::Value, _> = serde_json::from_str(raw_json);
210    match parsed {
211        Ok(json) => {
212            if let Some(messages) = json.get("messages").and_then(|m| m.as_array()) {
213                // Find the last assistant message
214                for msg in messages.iter().rev() {
215                    if msg.get("role").and_then(|r| r.as_str()) == Some("assistant") {
216                        if let Some(content) = msg.get("content").and_then(|c| c.as_array()) {
217                            // Collect all text blocks
218                            let texts: Vec<&str> = content
219                                .iter()
220                                .filter_map(|c| {
221                                    if c.get("type").and_then(|t| t.as_str()) == Some("text") {
222                                        c.get("text").and_then(|t| t.as_str())
223                                    } else {
224                                        None
225                                    }
226                                })
227                                .collect();
228                            if !texts.is_empty() {
229                                return texts.join("\n");
230                            }
231                        }
232                    }
233                }
234                // Valid JSON with messages array but no assistant text —
235                // goose ran but produced no output. Return empty so the
236                // retry logic can detect this and retry.
237                return String::new();
238            }
239            // No messages array at all — not goose JSON format.
240            // Return as-is (might be plain text from older goose).
241            raw_json.to_string()
242        }
243        Err(_) => {
244            // Not valid JSON — return as-is (might be plain text from older goose)
245            raw_json.to_string()
246        }
247    }
248}
249
250/// Extract the "## Changes Applied" section from the LLM's response.
251///
252/// The prompt instructs the LLM to produce this section after writing the file.
253/// We extract it verbatim to pass as continuation context to the next chunk,
254/// so subsequent chunks know what was actually changed (not just what was requested).
255///
256/// Falls back to "## Summary of Changes", "## Summary of changes", or
257/// "## Changes Applied" variants for robustness.
258fn extract_changes_applied(response: &str) -> Option<String> {
259    // Try multiple header patterns the LLM might use
260    let markers = [
261        "## Changes Applied",
262        "## Summary of Changes",
263        "## Summary of changes",
264        "## Summary",
265    ];
266
267    for marker in &markers {
268        if let Some(start) = response.find(marker) {
269            let section = &response[start..];
270            // Trim to just this section — stop at the next top-level heading
271            // or end of response.
272            let end = section[marker.len()..]
273                .find("\n## ")
274                .map(|pos| marker.len() + pos)
275                .unwrap_or(section.len());
276            let trimmed = section[..end].trim();
277            if !trimmed.is_empty() {
278                return Some(trimmed.to_string());
279            }
280        }
281    }
282
283    None
284}
285
286/// Maximum number of files to process concurrently.
287/// Each file spawns a goose process, so this limits system load.
288const MAX_CONCURRENT_FILES: usize = 3;
289
290pub fn run_all_goose_fixes(
291    requests: &[LlmFixRequest],
292    ctx: &dyn FixContext,
293    verbose: bool,
294    log_dir: Option<&std::path::Path>,
295) -> Vec<GooseFixResult> {
296    // Create log directory if specified
297    if let Some(dir) = log_dir {
298        let _ = std::fs::create_dir_all(dir);
299    }
300
301    // Group by file path for batching
302    let mut by_file: BTreeMap<PathBuf, Vec<&LlmFixRequest>> = BTreeMap::new();
303    for req in requests {
304        by_file.entry(req.file_path.clone()).or_default().push(req);
305    }
306
307    // Merge incidents from the same rule within each file, then sort by
308    // priority so the most impactful structural migration rules (hierarchy
309    // composition) come first in each batch. This ensures:
310    //  1. Multiple incidents from the same rule are presented as one fix.
311    //  2. The first chunk starts with structural migration rules that
312    //     trigger tool calls (file reads/edits), preventing empty goose output.
313    //  3. Informational/review-only rules come last where they're less likely
314    //     to consume turns or confuse the LLM.
315    let mut merged_by_file: Vec<(PathBuf, Vec<MergedLlmFixRequest>)> = Vec::new();
316    for (path, file_reqs) in by_file {
317        let mut merged = merge_by_rule_id(&file_reqs);
318        // Sort family rules first (they represent coherent migrations that
319        // should be processed as early as possible), grouped by family name
320        // so same-family rules are adjacent for chunking. Within each group,
321        // sort by individual priority. Non-family rules come after.
322        merged.sort_by(|a, b| {
323            let a_has_family = a.family.is_some();
324            let b_has_family = b.family.is_some();
325            match (a_has_family, b_has_family) {
326                (true, false) => std::cmp::Ordering::Less,
327                (false, true) => std::cmp::Ordering::Greater,
328                (true, true) => a.family.cmp(&b.family).then_with(|| {
329                    ctx.fix_priority(&a.rule_id)
330                        .cmp(&ctx.fix_priority(&b.rule_id))
331                }),
332                (false, false) => ctx
333                    .fix_priority(&a.rule_id)
334                    .cmp(&ctx.fix_priority(&b.rule_id)),
335            }
336        });
337        merged_by_file.push((path, merged));
338    }
339
340    let total_files = merged_by_file.len();
341    let total_fixes = requests.len();
342    eprintln!(
343        "  Processing {} fixes across {} files via goose ({} concurrent)...\n",
344        total_fixes, total_files, MAX_CONCURRENT_FILES
345    );
346
347    let pipeline_start = std::time::Instant::now();
348    let completed = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0));
349    let succeeded = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0));
350    let failed_count = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0));
351
352    // Use a channel as a simple concurrency limiter (semaphore)
353    let (sem_tx, sem_rx) = std::sync::mpsc::sync_channel::<()>(MAX_CONCURRENT_FILES);
354    for _ in 0..MAX_CONCURRENT_FILES {
355        sem_tx.send(()).unwrap();
356    }
357
358    let file_entries: Vec<(usize, PathBuf, Vec<MergedLlmFixRequest>)> = merged_by_file
359        .into_iter()
360        .enumerate()
361        .map(|(i, (path, reqs))| (i, path, reqs))
362        .collect();
363
364    let results: Vec<GooseFixResult> = std::thread::scope(|s| {
365        let mut handles = Vec::new();
366
367        for (i, file_path, file_requests) in &file_entries {
368            // Acquire semaphore slot (blocks until a slot is free)
369            sem_rx.recv().unwrap();
370
371            let sem_tx = sem_tx.clone();
372            let done = completed.clone();
373            let ok_count = succeeded.clone();
374            let fail_count = failed_count.clone();
375            let i = *i;
376
377            let handle = s.spawn(move || {
378                let result = process_single_file(
379                    i,
380                    total_files,
381                    file_path,
382                    file_requests,
383                    ctx,
384                    verbose,
385                    log_dir,
386                );
387
388                let idx = done.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + 1;
389                match &result {
390                    r if r.success => {
391                        ok_count.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
392                    }
393                    _ => {
394                        fail_count.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
395                    }
396                }
397                eprintln!("  [{}/{}] complete", idx, total_files,);
398
399                // Release semaphore slot
400                let _ = sem_tx.send(());
401                result
402            });
403
404            handles.push(handle);
405        }
406
407        handles.into_iter().map(|h| h.join().unwrap()).collect()
408    });
409
410    let total_elapsed = pipeline_start.elapsed();
411    let ok = succeeded.load(std::sync::atomic::Ordering::Relaxed);
412    let fail = failed_count.load(std::sync::atomic::Ordering::Relaxed);
413    eprintln!(
414        "  Goose complete: {} succeeded, {} failed ({:.0}s total, {:.1}s avg per file)",
415        ok,
416        fail,
417        total_elapsed.as_secs_f64(),
418        total_elapsed.as_secs_f64() / total_files.max(1) as f64,
419    );
420
421    results
422}
423
424/// Process all fixes for a single file. Chunks are processed sequentially
425/// within the file (each chunk reads the file as modified by the previous).
426/// This function is called from parallel threads — one per file.
427fn process_single_file(
428    file_index: usize,
429    total_files: usize,
430    file_path: &std::path::Path,
431    file_requests: &[MergedLlmFixRequest],
432    ctx: &dyn FixContext,
433    verbose: bool,
434    log_dir: Option<&std::path::Path>,
435) -> GooseFixResult {
436    let file_name = file_path
437        .file_name()
438        .map(|n| n.to_string_lossy().to_string())
439        .unwrap_or_else(|| file_path.display().to_string());
440
441    let rule_ids: Vec<&str> = file_requests.iter().map(|r| r.rule_id.as_str()).collect();
442    let rules_display = if rule_ids.len() <= 3 {
443        rule_ids.join(", ")
444    } else {
445        format!(
446            "{}, ... +{} more",
447            rule_ids[..2].join(", "),
448            rule_ids.len() - 2
449        )
450    };
451
452    eprintln!(
453        "  [{}/{}] {} ({} fixes) [{}]",
454        file_index + 1,
455        total_files,
456        file_name,
457        file_requests.len(),
458        rules_display,
459    );
460
461    let file_start = std::time::Instant::now();
462
463    // Split large batches into chunks to avoid overwhelming the LLM.
464    // Each chunk runs sequentially — the LLM reads the file as modified
465    // by the previous chunk. A context summary of previously applied
466    // fixes is prepended to each subsequent chunk.
467    let max_fixes_per_batch = 8;
468    let mut result: Result<GooseFixResult> = Ok(GooseFixResult {
469        file_path: file_path.to_path_buf(),
470        rule_id: String::new(),
471        success: true,
472        output: String::new(),
473    });
474    let mut all_prompts = Vec::new();
475    let mut all_outputs: Vec<String> = Vec::new();
476    let mut all_stderrs: Vec<String> = Vec::new();
477    let mut chunk_times: Vec<f64> = Vec::new();
478    let mut chunk_retried: Vec<bool> = Vec::new();
479    let mut applied_summaries: Vec<String> = Vec::new();
480
481    if file_requests.len() == 1 {
482        let prompt = build_merged_prompt(&file_requests[0], ctx);
483        all_prompts.push(prompt.clone());
484        let max_turns_str = "5".to_string();
485        let mut goose_result = run_goose_with_timeout(&prompt, &max_turns_str);
486        let mut was_retried = false;
487        // Retry once on empty response
488        if let Ok((_, ref output, _)) = goose_result {
489            if output.len() <= 1 {
490                was_retried = true;
491                eprintln!("         {}: empty response — retrying once...", file_name,);
492                std::thread::sleep(Duration::from_secs(2));
493                goose_result = run_goose_with_timeout(&prompt, &max_turns_str);
494            }
495        }
496        match goose_result {
497            Ok((success, output, stderr)) => {
498                all_outputs.push(output.clone());
499                all_stderrs.push(stderr);
500                chunk_times.push(0.0);
501                chunk_retried.push(was_retried);
502                result = Ok(GooseFixResult {
503                    file_path: file_path.to_path_buf(),
504                    rule_id: file_requests[0].rule_id.clone(),
505                    success,
506                    output,
507                });
508            }
509            Err(e) => {
510                result = Err(e);
511            }
512        }
513    } else {
514        // Build family-aware chunks. Rules from the same component family
515        // must stay in the same chunk so the LLM sees the full migration
516        // (composition + prop→child + conformance) as one coherent change.
517        // A family group is treated as one logical unit regardless of size.
518        let chunks: Vec<Vec<&MergedLlmFixRequest>> = {
519            let mut result: Vec<Vec<&MergedLlmFixRequest>> = Vec::new();
520            let mut current_chunk: Vec<&MergedLlmFixRequest> = Vec::new();
521            let mut current_families: std::collections::HashSet<String> =
522                std::collections::HashSet::new();
523
524            for req in file_requests.iter() {
525                if let Some(ref fam) = req.family {
526                    if current_families.contains(fam) {
527                        // Same family — always add to current chunk
528                        current_chunk.push(req);
529                    } else if current_chunk.len() >= max_fixes_per_batch
530                        && !current_chunk.is_empty()
531                    {
532                        // New family and chunk is full — start new chunk
533                        result.push(std::mem::take(&mut current_chunk));
534                        current_families.clear();
535                        current_families.insert(fam.clone());
536                        current_chunk.push(req);
537                    } else {
538                        // New family, chunk has room
539                        current_families.insert(fam.clone());
540                        current_chunk.push(req);
541                    }
542                } else {
543                    // No family — add to current chunk, respect size limit
544                    if current_chunk.len() >= max_fixes_per_batch {
545                        result.push(std::mem::take(&mut current_chunk));
546                        current_families.clear();
547                    }
548                    current_chunk.push(req);
549                }
550            }
551            if !current_chunk.is_empty() {
552                result.push(current_chunk);
553            }
554            result
555        };
556        let chunk_count = chunks.len();
557
558        for (chunk_idx, chunk) in chunks.iter().enumerate() {
559            if chunk_idx > 0 {
560                eprintln!(
561                    "         {}: chunk {}/{} ({} fixes)...",
562                    file_name,
563                    chunk_idx + 1,
564                    chunk_count,
565                    chunk.len()
566                );
567            }
568
569            let chunk_refs: Vec<&MergedLlmFixRequest> = chunk.to_vec();
570
571            let prompt = build_batch_prompt_with_context(
572                file_path,
573                &chunk_refs,
574                if applied_summaries.is_empty() {
575                    None
576                } else {
577                    Some(&applied_summaries)
578                },
579                ctx,
580            );
581            all_prompts.push(prompt.clone());
582
583            let max_turns = (22 + chunk.len()).min(40);
584            let max_turns_str = max_turns.to_string();
585            let chunk_start = std::time::Instant::now();
586            let mut chunk_result = run_goose_with_timeout(&prompt, &max_turns_str);
587            let mut was_retried = false;
588
589            // Retry once on empty response. Goose sometimes returns an
590            // empty assistant message (no tool calls, no text) due to
591            // transient LLM API issues or serialization failures.
592            // NOTE: The first attempt may have made PARTIAL edits to the
593            // file before failing to produce a summary. The retry prompt
594            // accounts for this.
595            if let Ok((_, ref output, _)) = chunk_result {
596                if output.len() <= 1 {
597                    was_retried = true;
598                    eprintln!(
599                        "         {}: chunk {}/{} empty response — retrying once...",
600                        file_name,
601                        chunk_idx + 1,
602                        chunk_count,
603                    );
604                    std::thread::sleep(Duration::from_secs(2));
605                    let retry_prompt = format!(
606                        "{}\n\n\
607                         RETRY: The previous attempt may have made PARTIAL changes to the file but did not complete. \
608                         You MUST read the file as it exists NOW on disk, check each fix individually, \
609                         and apply every fix that is not yet present. Do not assume all fixes are applied \
610                         just because some are — check EVERY one.",
611                        prompt,
612                    );
613                    chunk_result = run_goose_with_timeout(&retry_prompt, &max_turns_str);
614                }
615            }
616
617            let chunk_elapsed = chunk_start.elapsed();
618
619            match chunk_result {
620                Ok((success, output, stderr)) => {
621                    let resp_len = output.len();
622                    let status = if resp_len <= 1 {
623                        "EMPTY"
624                    } else if success {
625                        "ok"
626                    } else {
627                        "FAILED"
628                    };
629                    // Count messages in the goose JSON to understand what happened
630                    let msg_count = serde_json::from_str::<serde_json::Value>(&stderr)
631                        .ok()
632                        .and_then(|j| j.get("messages")?.as_array().map(|a| a.len()))
633                        .unwrap_or(0);
634
635                    eprintln!(
636                        "         {}: chunk {}/{} {} ({} fixes, {:.1}s, response={} chars, goose_messages={})",
637                        file_name,
638                        chunk_idx + 1,
639                        chunk_count,
640                        status,
641                        chunk.len(),
642                        chunk_elapsed.as_secs_f64(),
643                        resp_len,
644                        msg_count,
645                    );
646                    // If empty response, summarize what goose did from the JSON
647                    if resp_len <= 1 {
648                        if let Ok(json) = serde_json::from_str::<serde_json::Value>(&stderr) {
649                            if let Some(messages) = json.get("messages").and_then(|m| m.as_array())
650                            {
651                                for msg in messages {
652                                    let role =
653                                        msg.get("role").and_then(|r| r.as_str()).unwrap_or("?");
654                                    if role == "assistant" {
655                                        if let Some(content) =
656                                            msg.get("content").and_then(|c| c.as_array())
657                                        {
658                                            for item in content {
659                                                let typ = item
660                                                    .get("type")
661                                                    .and_then(|t| t.as_str())
662                                                    .unwrap_or("?");
663                                                if typ == "toolUse" {
664                                                    let tool = item
665                                                        .get("name")
666                                                        .and_then(|n| n.as_str())
667                                                        .unwrap_or("?");
668                                                    eprintln!(
669                                                        "           goose: tool_call={}",
670                                                        tool
671                                                    );
672                                                } else if typ == "text" {
673                                                    let text = item
674                                                        .get("text")
675                                                        .and_then(|t| t.as_str())
676                                                        .unwrap_or("");
677                                                    if !text.is_empty() {
678                                                        eprintln!(
679                                                            "           goose: text={}",
680                                                            &text[..text.len().min(100)]
681                                                        );
682                                                    }
683                                                }
684                                            }
685                                        }
686                                    }
687                                }
688                            }
689                        } else if msg_count == 0 {
690                            eprintln!("           goose: no messages in output (goose may have failed silently)");
691                        }
692                    }
693
694                    chunk_times.push(chunk_elapsed.as_secs_f64());
695                    chunk_retried.push(was_retried);
696                    all_stderrs.push(stderr);
697
698                    // Record what was applied for context in next chunk.
699                    // Extract the "## Changes Applied" section from the LLM's
700                    // response, which describes what was actually done (or not).
701                    // This is more useful than echoing the fix descriptions,
702                    // because it tells the next chunk what the file looks like
703                    // now, not what was requested.
704                    if let Some(summary) = extract_changes_applied(&output) {
705                        applied_summaries.push(summary);
706                    } else {
707                        // Fallback: use first line of each fix description
708                        for req in chunk.iter() {
709                            let lines_display = req
710                                .lines
711                                .iter()
712                                .map(|l| l.to_string())
713                                .collect::<Vec<_>>()
714                                .join(", ");
715                            let first_line =
716                                req.message.lines().next().unwrap_or("(no description)");
717                            applied_summaries.push(format!(
718                                "- {} (line {}): {}",
719                                req.rule_id, lines_display, first_line
720                            ));
721                        }
722                    }
723                    all_outputs.push(output.clone());
724                    result = Ok(GooseFixResult {
725                        file_path: file_path.to_path_buf(),
726                        rule_id: file_requests
727                            .iter()
728                            .map(|r| r.rule_id.as_str())
729                            .collect::<Vec<_>>()
730                            .join(", "),
731                        success,
732                        output,
733                    });
734                    if !success {
735                        eprintln!(
736                            "         {}: chunk {}/{} FAILED, stopping",
737                            file_name,
738                            chunk_idx + 1,
739                            chunk_count
740                        );
741                        break;
742                    }
743                }
744                Err(e) => {
745                    // Retry on timeout
746                    let err_msg = format!("{}", e);
747                    if err_msg.contains("timed out") {
748                        let backoff = Duration::from_secs(10);
749                        eprintln!(
750                            "         {}: chunk {}/{} timed out after {:.1}s, retrying in {}s...",
751                            file_name,
752                            chunk_idx + 1,
753                            chunk_count,
754                            chunk_elapsed.as_secs_f64(),
755                            backoff.as_secs(),
756                        );
757                        std::thread::sleep(backoff);
758                        let _retry_start = std::time::Instant::now();
759                        let retry_result = run_goose_with_timeout(&prompt, &max_turns_str);
760                        match retry_result {
761                            Ok((success, output, _retry_stderr)) => {
762                                for req in chunk.iter() {
763                                    let lines_display = req
764                                        .lines
765                                        .iter()
766                                        .map(|l| l.to_string())
767                                        .collect::<Vec<_>>()
768                                        .join(", ");
769                                    let summary: String = req
770                                        .message
771                                        .lines()
772                                        .take(3)
773                                        .collect::<Vec<_>>()
774                                        .join("\n  ");
775                                    applied_summaries.push(format!(
776                                        "- {} (line {}): {}",
777                                        req.rule_id, lines_display, summary
778                                    ));
779                                }
780                                all_outputs.push(output.clone());
781                                result = Ok(GooseFixResult {
782                                    file_path: file_path.to_path_buf(),
783                                    rule_id: file_requests
784                                        .iter()
785                                        .map(|r| r.rule_id.as_str())
786                                        .collect::<Vec<_>>()
787                                        .join(", "),
788                                    success,
789                                    output,
790                                });
791                            }
792                            Err(e2) => {
793                                result = Err(e2);
794                                break;
795                            }
796                        }
797                    } else {
798                        result = Err(e);
799                        break;
800                    }
801                }
802            }
803        }
804    }
805
806    let elapsed = file_start.elapsed();
807
808    match result {
809        Ok(r) => {
810            if r.success {
811                eprintln!("         {}: ok ({:.1}s)", file_name, elapsed.as_secs_f64());
812            } else {
813                eprintln!(
814                    "         {}: FAILED ({:.1}s)",
815                    file_name,
816                    elapsed.as_secs_f64()
817                );
818            }
819            if verbose && !r.output.is_empty() {
820                for line in r.output.lines().take(5) {
821                    eprintln!("           {}", line);
822                }
823            }
824
825            // Save all prompts + responses to log file (one entry per chunk)
826            if let Some(dir) = log_dir {
827                let chunks: Vec<serde_json::Value> = all_prompts
828                    .iter()
829                    .enumerate()
830                    .map(|(i, prompt)| {
831                        let resp = all_outputs.get(i).unwrap_or(&String::new()).clone();
832                        let raw_json = all_stderrs.get(i).unwrap_or(&String::new()).clone();
833                        let resp_len = resp.len();
834                        // Parse the raw goose JSON for structured logging
835                        let goose_session: serde_json::Value = serde_json::from_str(&raw_json)
836                            .unwrap_or_else(|_| serde_json::json!({"raw": raw_json}));
837                        serde_json::json!({
838                            "chunk": i + 1,
839                            "prompt": prompt,
840                            "response": resp,
841                            "response_length": resp_len,
842                            "elapsed_secs": chunk_times.get(i).unwrap_or(&0.0),
843                            "retried": chunk_retried.get(i).unwrap_or(&false),
844                            "status": if resp_len <= 1 { "empty" } else { "ok" },
845                            "goose_session": goose_session,
846                        })
847                    })
848                    .collect();
849
850                let log_entry = serde_json::json!({
851                    "file": file_path.display().to_string(),
852                    "rule_ids": file_requests.iter().map(|r| &r.rule_id).collect::<Vec<_>>(),
853                    "chunks": chunks,
854                    "total_chunks": all_prompts.len(),
855                    "success": r.success,
856                    "elapsed_secs": elapsed.as_secs_f64(),
857                });
858                let log_file = dir.join(format!("goose-fix-{:03}.json", file_index + 1));
859                let _ = std::fs::write(
860                    &log_file,
861                    serde_json::to_string_pretty(&log_entry).unwrap_or_default(),
862                );
863            }
864
865            r
866        }
867        Err(e) => {
868            eprintln!(
869                "         {}: ERROR ({:.1}s) — {}",
870                file_name,
871                elapsed.as_secs_f64(),
872                e
873            );
874            GooseFixResult {
875                file_path: file_path.to_path_buf(),
876                rule_id: file_requests
877                    .iter()
878                    .map(|r| r.rule_id.as_str())
879                    .collect::<Vec<_>>()
880                    .join(", "),
881                success: false,
882                output: format!("Error: {}", e),
883            }
884        }
885    }
886}
887
888// ── Prompt construction ───────────────────────────────────────────────────
889
890/// Build a prompt for a single merged fix request (one unique rule, possibly
891/// multiple incident lines).
892fn build_merged_prompt(request: &MergedLlmFixRequest, ctx: &dyn FixContext) -> String {
893    let lines_display = request
894        .lines
895        .iter()
896        .map(|l| l.to_string())
897        .collect::<Vec<_>>()
898        .join(", ");
899
900    let mut code_context = String::new();
901    if request.code_snips.is_empty() {
902        code_context.push_str("(no code snippet available)");
903    } else if request.code_snips.len() == 1 {
904        code_context.push_str(&request.code_snips[0].1);
905    } else {
906        for (line, snip) in &request.code_snips {
907            code_context.push_str(&format!("  (line {}):\n{}\n", line, snip));
908        }
909    }
910
911    let constraints = ctx.llm_constraints();
912    let constraints_section = if constraints.is_empty() {
913        String::new()
914    } else {
915        let lines: Vec<String> = constraints.iter().map(|c| format!("- {}", c)).collect();
916        format!("\nIMPORTANT constraints:\n{}", lines.join("\n"))
917    };
918
919    format!(
920        r#"You are applying a {migration_desc} fix.
921
922File: {file_path}
923Line: {lines}
924
925Migration rule [{rule_id}]:
926{message}
927
928Code context:
929```
930{code_context}
931```
932
933Instructions:
9341. Read the file at {file_path}
9352. Apply ONLY the change described by the migration rule at or near line {lines}
9363. Make the minimum edit necessary — do not change unrelated code, but DO clean up any artifacts caused by your change (e.g., remove imports that are no longer referenced, delete dead declarations)
9374. Write the fixed file
938{constraints_section}
939
940Before writing, reason through the fix step by step to ensure nothing is missed. Then read the file, make the edit, and write it.
941
942After writing the file, produce a '## Changes Applied' section that lists the change you made, or note if the fix was already applied or could not be applied (with a brief reason)."#,
943        migration_desc = ctx.migration_description(),
944        file_path = request.file_path.display(),
945        lines = lines_display,
946        rule_id = request.rule_id,
947        message = request.message,
948        code_context = code_context,
949        constraints_section = constraints_section,
950    )
951}
952
953/// Format a single fix entry in the batch prompt.
954fn format_fix_entry(fixes: &mut String, fix_num: usize, req: &MergedLlmFixRequest) {
955    let lines_display = req
956        .lines
957        .iter()
958        .map(|l| l.to_string())
959        .collect::<Vec<_>>()
960        .join(", ");
961
962    if req.lines.len() == 1 {
963        let code_context = req
964            .code_snips
965            .first()
966            .map(|(_, s)| s.as_str())
967            .unwrap_or("(no snippet)");
968        fixes.push_str(&format!(
969            r#"
970### Fix {num}
971Line: {line}
972Rule [{rule_id}]:
973{message}
974
975Code context:
976```
977{code_context}
978```
979"#,
980            num = fix_num,
981            line = lines_display,
982            rule_id = req.rule_id,
983            message = req.message,
984            code_context = code_context,
985        ));
986    } else {
987        let mut all_snippets = String::new();
988        for (line, snip) in &req.code_snips {
989            all_snippets.push_str(&format!("  (line {}):\n{}\n", line, snip));
990        }
991        fixes.push_str(&format!(
992            r#"
993### Fix {num}
994Lines: {lines}
995Rule [{rule_id}]:
996{message}
997
998This rule affects multiple locations in the file. Apply ALL steps together as one logical change.
999
1000Code contexts:
1001```
1002{all_snippets}```
1003"#,
1004            num = fix_num,
1005            lines = lines_display,
1006            rule_id = req.rule_id,
1007            message = req.message,
1008            all_snippets = all_snippets,
1009        ));
1010    }
1011}
1012
1013fn build_batch_prompt_with_context(
1014    file_path: &std::path::Path,
1015    requests: &[&MergedLlmFixRequest],
1016    previously_applied: Option<&[String]>,
1017    ctx: &dyn FixContext,
1018) -> String {
1019    // Group requests by component family so the LLM sees related rules
1020    // as one coherent migration (e.g., all Modal prop→child + composition
1021    // rules together) rather than independent fixes.
1022    let mut fixes = String::new();
1023    let mut fix_num = 0usize;
1024
1025    // Partition into family-grouped and ungrouped
1026    let mut family_groups: std::collections::BTreeMap<String, Vec<&MergedLlmFixRequest>> =
1027        std::collections::BTreeMap::new();
1028    let mut ungrouped: Vec<&MergedLlmFixRequest> = Vec::new();
1029
1030    for req in requests.iter() {
1031        if let Some(ref fam) = req.family {
1032            family_groups.entry(fam.clone()).or_default().push(req);
1033        } else {
1034            ungrouped.push(req);
1035        }
1036    }
1037
1038    // Emit family-grouped fixes first (they're higher priority structurally)
1039    for (family, group) in &family_groups {
1040        if group.len() > 1 {
1041            fixes.push_str(&format!(
1042                "\n## {} Migration (apply as ONE coherent change)\n\
1043                 The following {} rules are all part of the {} component family migration.\n\
1044                 Apply them together — they describe different aspects of the same restructuring.\n",
1045                family,
1046                group.len(),
1047                family,
1048            ));
1049        }
1050
1051        for req in group {
1052            fix_num += 1;
1053            format_fix_entry(&mut fixes, fix_num, req);
1054        }
1055    }
1056
1057    // Then emit ungrouped fixes
1058    for req in &ungrouped {
1059        fix_num += 1;
1060        format_fix_entry(&mut fixes, fix_num, req);
1061    }
1062
1063    let revert_warning = ctx.revert_warnings().unwrap_or("");
1064    let context_section = if let Some(applied) = previously_applied {
1065        let mut section = "\n## Changes from previous pass:\n\
1066             The following changes were made in a previous pass and are already applied\n\
1067             to the file on disk. Do NOT revert these changes.\n"
1068            .to_string();
1069        if !revert_warning.is_empty() {
1070            section.push_str(revert_warning);
1071            section.push('\n');
1072        }
1073        section.push_str(
1074            "If any listed change was NOT actually applied (the old pattern still exists\n\
1075             in the file), apply it now along with the new fixes below.\n\n",
1076        );
1077        section.push_str(&applied.join("\n"));
1078        section.push_str("\n\n");
1079        section
1080    } else {
1081        String::new()
1082    };
1083
1084    let constraints = ctx.llm_constraints();
1085    let constraints_section = if constraints.is_empty() {
1086        String::new()
1087    } else {
1088        let lines: Vec<String> = constraints.iter().map(|c| format!("- {}", c)).collect();
1089        format!("\nIMPORTANT constraints:\n{}", lines.join("\n"))
1090    };
1091
1092    let verification_section = ctx
1093        .verification_prompt()
1094        .map(|v| format!("\n{}\n", v))
1095        .unwrap_or_default();
1096
1097    format!(
1098        r#"You are applying {migration_desc} fixes to a single file.
1099
1100File: {file_path}
1101{context_section}
1102Apply ALL of the following {count} fixes to this file:
1103{fixes}
1104Instructions:
11051. Read the file at {file_path}
11062. Process each fix INDEPENDENTLY in sequence. For each fix:
1107   a. Identify the exact code affected (line number and affected element)
1108   b. Determine the specific change needed ({change_examples})
1109   c. Track all changes for the final write
11103. Make the minimum edits necessary — do not change unrelated code, but DO clean up any artifacts caused by your changes (e.g., remove imports that are no longer referenced, delete dead declarations)
11114. Do NOT revert any changes that were already applied in previous passes
11125. Write the fixed file once with ALL changes from every fix applied
1113{constraints_section}
1114{verification_section}
1115Before writing, reason through each fix step by step to ensure nothing is missed. Then read the file, make the edits, and write it.
1116
1117After writing the file, produce a '## Changes Applied' section that lists each change you made, each fix that was already applied (no change needed), and each fix you could not apply (with a brief reason). This summary is used by subsequent processing steps."#,
1118        migration_desc = ctx.migration_description(),
1119        file_path = file_path.display(),
1120        context_section = context_section,
1121        count = requests.len(),
1122        fixes = fixes,
1123        constraints_section = constraints_section,
1124        change_examples = ctx.change_type_examples(),
1125        verification_section = verification_section,
1126    )
1127}
1128
1129#[cfg(test)]
1130mod tests {
1131    use super::*;
1132    use std::path::PathBuf;
1133
1134    fn make_req(rule_id: &str) -> LlmFixRequest {
1135        make_req_at_line(rule_id, 1, None)
1136    }
1137
1138    fn make_req_at_line(rule_id: &str, line: u32, code_snip: Option<&str>) -> LlmFixRequest {
1139        LlmFixRequest {
1140            file_path: PathBuf::from("/tmp/test.tsx"),
1141            file_uri: "file:///tmp/test.tsx".to_string(),
1142            line,
1143            rule_id: rule_id.to_string(),
1144            message: format!("Migration for {}", rule_id),
1145            code_snip: code_snip.map(|s| s.to_string()),
1146            source: None,
1147            labels: Vec::new(),
1148        }
1149    }
1150
1151    // NOTE: fix_priority tests have moved to the patternfly-fix-context crate,
1152    // since priority ordering is now a FixContext concern, not a goose_client one.
1153
1154    // ── merge_by_rule_id tests ───────────────────────────────────────────
1155
1156    #[test]
1157    fn test_merge_by_rule_id_no_duplicates() {
1158        let reqs = vec![make_req("rule-a"), make_req("rule-b"), make_req("rule-c")];
1159        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1160        let merged = merge_by_rule_id(&refs);
1161
1162        assert_eq!(merged.len(), 3);
1163        assert_eq!(merged[0].rule_id, "rule-a");
1164        assert_eq!(merged[1].rule_id, "rule-b");
1165        assert_eq!(merged[2].rule_id, "rule-c");
1166        assert_eq!(merged[0].lines, vec![1]);
1167        assert_eq!(merged[1].lines, vec![1]);
1168        assert_eq!(merged[2].lines, vec![1]);
1169    }
1170
1171    #[test]
1172    fn test_merge_by_rule_id_combines_same_rule() {
1173        let reqs = vec![
1174            make_req_at_line("rule-a", 7, Some("line 7 code")),
1175            make_req_at_line("rule-b", 10, None),
1176            make_req_at_line("rule-a", 152, Some("line 152 code")),
1177        ];
1178        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1179        let merged = merge_by_rule_id(&refs);
1180
1181        assert_eq!(merged.len(), 2);
1182        // rule-a appears first (first occurrence)
1183        assert_eq!(merged[0].rule_id, "rule-a");
1184        assert_eq!(merged[0].lines, vec![7, 152]);
1185        assert_eq!(merged[0].code_snips.len(), 2);
1186        assert_eq!(merged[0].code_snips[0], (7, "line 7 code".to_string()));
1187        assert_eq!(merged[0].code_snips[1], (152, "line 152 code".to_string()));
1188        // rule-b is second
1189        assert_eq!(merged[1].rule_id, "rule-b");
1190        assert_eq!(merged[1].lines, vec![10]);
1191    }
1192
1193    #[test]
1194    fn test_merge_preserves_insertion_order() {
1195        // Simulate the order after priority sort: hierarchy first, then
1196        // composition, then prop-level, then conformance.
1197        let reqs = vec![
1198            make_req_at_line("semver-hierarchy-modal-composition-changed", 9, None),
1199            make_req_at_line("semver-hierarchy-emptystate-composition-changed", 6, None),
1200            make_req_at_line("semver-composition-button-children-to-icon-prop", 139, None),
1201            make_req_at_line(
1202                "semver-composition-emptystateheader-nesting-changed",
1203                152,
1204                None,
1205            ),
1206            make_req_at_line(
1207                "semver-emptystateheader-component-import-deprecated",
1208                7,
1209                None,
1210            ),
1211            make_req_at_line(
1212                "semver-composition-emptystateheader-nesting-changed",
1213                7,
1214                None,
1215            ), // dup at different line
1216            make_req_at_line("conformance-table-expected-children", 14, None),
1217        ];
1218        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1219        let merged = merge_by_rule_id(&refs);
1220
1221        // 6 unique rules (emptystateheader-nesting-changed merges two lines)
1222        assert_eq!(merged.len(), 6);
1223        // Verify order matches insertion (priority) order
1224        assert_eq!(
1225            merged[0].rule_id,
1226            "semver-hierarchy-modal-composition-changed"
1227        );
1228        assert_eq!(
1229            merged[1].rule_id,
1230            "semver-hierarchy-emptystate-composition-changed"
1231        );
1232        assert_eq!(
1233            merged[2].rule_id,
1234            "semver-composition-button-children-to-icon-prop"
1235        );
1236        assert_eq!(
1237            merged[3].rule_id,
1238            "semver-composition-emptystateheader-nesting-changed"
1239        );
1240        assert_eq!(merged[3].lines, vec![152, 7]); // both lines preserved
1241        assert_eq!(
1242            merged[4].rule_id,
1243            "semver-emptystateheader-component-import-deprecated"
1244        );
1245        assert_eq!(merged[5].rule_id, "conformance-table-expected-children");
1246    }
1247
1248    #[test]
1249    fn test_merge_then_sort_with_context() {
1250        // Tests that merge + sort works with a FixContext.
1251        // With GenericFixContext (priority 3 for all), insertion order is preserved.
1252        let reqs = vec![
1253            make_req_at_line("rule-a", 10, None),
1254            make_req_at_line("rule-b", 20, None),
1255            make_req_at_line("rule-c", 30, None),
1256        ];
1257        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1258
1259        let mut merged = merge_by_rule_id(&refs);
1260        let ctx = crate::context::GenericFixContext;
1261        merged.sort_by(|a, b| {
1262            ctx.fix_priority(&a.rule_id)
1263                .cmp(&ctx.fix_priority(&b.rule_id))
1264        });
1265
1266        // With equal priority, sort is stable — insertion order preserved
1267        assert_eq!(merged.len(), 3);
1268        assert_eq!(merged[0].rule_id, "rule-a");
1269        assert_eq!(merged[1].rule_id, "rule-b");
1270        assert_eq!(merged[2].rule_id, "rule-c");
1271    }
1272
1273    #[test]
1274    fn test_batch_prompt_includes_all_fixes() {
1275        // Verify that the batch prompt includes all fix entries.
1276        let reqs = vec![
1277            make_req("rule-alpha"),
1278            make_req("rule-beta"),
1279            make_req("rule-gamma"),
1280        ];
1281        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1282
1283        let merged = merge_by_rule_id(&refs);
1284        let merged_refs: Vec<&MergedLlmFixRequest> = merged.iter().collect();
1285        let ctx = crate::context::GenericFixContext;
1286        let prompt = build_batch_prompt_with_context(
1287            &PathBuf::from("/tmp/test.tsx"),
1288            &merged_refs,
1289            None,
1290            &ctx,
1291        );
1292
1293        // All rules appear in the prompt
1294        assert!(prompt.contains("rule-alpha"));
1295        assert!(prompt.contains("rule-beta"));
1296        assert!(prompt.contains("rule-gamma"));
1297        // Uses the generic migration description
1298        assert!(prompt.contains("code migration"));
1299    }
1300
1301    #[test]
1302    fn test_extract_text_from_goose_json_valid() {
1303        let json = r#"{
1304            "messages": [
1305                {"role": "user", "content": [{"type": "text", "text": "hello"}]},
1306                {"role": "assistant", "content": [{"type": "text", "text": "The file has been updated."}]}
1307            ]
1308        }"#;
1309        let result = extract_text_from_goose_json(json);
1310        assert_eq!(result, "The file has been updated.");
1311    }
1312
1313    #[test]
1314    fn test_extract_text_from_goose_json_empty_messages() {
1315        let json = r#"{"messages": []}"#;
1316        let result = extract_text_from_goose_json(json);
1317        // Returns empty so retry logic can detect the failure
1318        assert!(result.is_empty());
1319    }
1320
1321    #[test]
1322    fn test_extract_text_from_goose_json_user_only_no_assistant() {
1323        let json =
1324            r#"{"messages": [{"role": "user", "content": [{"type": "text", "text": "prompt"}]}]}"#;
1325        let result = extract_text_from_goose_json(json);
1326        // User message but no assistant response — return empty for retry
1327        assert!(result.is_empty());
1328    }
1329
1330    #[test]
1331    fn test_extract_text_from_goose_json_empty_object() {
1332        let json = "{}";
1333        let result = extract_text_from_goose_json(json);
1334        assert_eq!(result, "{}");
1335    }
1336
1337    #[test]
1338    fn test_extract_text_from_goose_json_not_json() {
1339        let text = "This is plain text output from goose";
1340        let result = extract_text_from_goose_json(text);
1341        assert_eq!(result, text);
1342    }
1343
1344    fn make_req_with_family(rule_id: &str, line: u32, family: &str) -> LlmFixRequest {
1345        let mut req = make_req_at_line(rule_id, line, None);
1346        req.labels.push(format!("family={}", family));
1347        req
1348    }
1349
1350    #[test]
1351    fn test_family_first_sort_groups_families_before_non_family() {
1352        // Family rules should sort before non-family rules regardless
1353        // of individual priority. Within families, same-family rules
1354        // cluster together.
1355        let reqs = vec![
1356            make_req_with_family("sd-composition-alert-requires-close", 10, "Alert"),
1357            make_req_at_line("semver-modal-component-import-deprecated", 3, None),
1358            make_req_with_family("sd-composition-modal-new-member-body", 20, "Modal"),
1359            make_req_with_family("sd-conformance-alert-close-in-group", 15, "Alert"),
1360            make_req_at_line("sd-conformance-pagesection-in-page", 40, None),
1361            make_req_with_family("sd-composition-modal-new-member-header", 25, "Modal"),
1362        ];
1363        let refs: Vec<&LlmFixRequest> = reqs.iter().collect();
1364
1365        let mut merged = merge_by_rule_id(&refs);
1366        // Use the family-first sort (same logic as run_all_goose_fixes)
1367        let ctx = crate::context::GenericFixContext;
1368        merged.sort_by(|a, b| {
1369            let a_has_family = a.family.is_some();
1370            let b_has_family = b.family.is_some();
1371            match (a_has_family, b_has_family) {
1372                (true, false) => std::cmp::Ordering::Less,
1373                (false, true) => std::cmp::Ordering::Greater,
1374                (true, true) => a.family.cmp(&b.family).then_with(|| {
1375                    ctx.fix_priority(&a.rule_id)
1376                        .cmp(&ctx.fix_priority(&b.rule_id))
1377                }),
1378                (false, false) => ctx
1379                    .fix_priority(&a.rule_id)
1380                    .cmp(&ctx.fix_priority(&b.rule_id)),
1381            }
1382        });
1383
1384        // All Alert rules first, then all Modal rules, then non-family
1385        assert_eq!(merged.len(), 6);
1386        assert_eq!(merged[0].family.as_deref(), Some("Alert"));
1387        assert_eq!(merged[1].family.as_deref(), Some("Alert"));
1388        assert_eq!(merged[2].family.as_deref(), Some("Modal"));
1389        assert_eq!(merged[3].family.as_deref(), Some("Modal"));
1390        assert!(merged[4].family.is_none());
1391        assert!(merged[5].family.is_none());
1392    }
1393
1394    #[test]
1395    fn test_extract_text_from_goose_json_multiple_text_blocks() {
1396        let json = r#"{
1397            "messages": [
1398                {"role": "assistant", "content": [
1399                    {"type": "text", "text": "First part."},
1400                    {"type": "toolUse", "name": "developer__read_file"},
1401                    {"type": "text", "text": "Second part."}
1402                ]}
1403            ]
1404        }"#;
1405        let result = extract_text_from_goose_json(json);
1406        assert_eq!(result, "First part.\nSecond part.");
1407    }
1408
1409    // ── extract_changes_applied tests ────────────────────────────────────
1410
1411    #[test]
1412    fn test_extract_changes_applied_standard_header() {
1413        let response = "Some reasoning...\n\n\
1414            ## Changes Applied\n\
1415            - Moved AlertActionCloseButton from actionClose prop to child of Alert\n\
1416            - Added ModalHeader, ModalBody, ModalFooter imports\n";
1417        let result = extract_changes_applied(response);
1418        assert!(result.is_some());
1419        let section = result.unwrap();
1420        assert!(section.starts_with("## Changes Applied"));
1421        assert!(section.contains("AlertActionCloseButton"));
1422        assert!(section.contains("ModalHeader"));
1423    }
1424
1425    #[test]
1426    fn test_extract_changes_applied_summary_of_changes_variant() {
1427        let response = "Analysis...\n\n\
1428            ## Summary of Changes\n\
1429            - Removed EmptyStateHeader\n\
1430            - Moved props to EmptyState\n";
1431        let result = extract_changes_applied(response);
1432        assert!(result.is_some());
1433        assert!(result.unwrap().contains("EmptyStateHeader"));
1434    }
1435
1436    #[test]
1437    fn test_extract_changes_applied_no_summary() {
1438        let response = "The file already follows the correct pattern. No changes needed.";
1439        let result = extract_changes_applied(response);
1440        assert!(result.is_none());
1441    }
1442
1443    #[test]
1444    fn test_extract_changes_applied_stops_at_next_heading() {
1445        let response = "## Changes Applied\n\
1446            - Fixed Modal composition\n\n\
1447            ## Additional Notes\n\
1448            Some extra info that should not be included.\n";
1449        let result = extract_changes_applied(response);
1450        assert!(result.is_some());
1451        let section = result.unwrap();
1452        assert!(section.contains("Fixed Modal"));
1453        assert!(!section.contains("Additional Notes"));
1454    }
1455}