From 05df2297a8e5fa5d7a782b3747d0ae255bac65c9 Mon Sep 17 00:00:00 2001 From: Andrew Thal <467872+athal7@users.noreply.github.com> Date: Thu, 9 Jul 2026 12:32:50 -0500 Subject: [PATCH 1/3] feat(explain): add --grouped diff summary mode Adds `lumen explain --grouped`, which clusters a diff into logical groups with a per-group AI summary plus an optional overall summary. - New grouped_summary module: JSON parsing (tolerant of markdown fences and stray prose), ground-truth file extraction from unified diff headers, reconciliation against hallucinated/missing files, and markdown rendering. - New AIPrompt::build_grouped_explain_prompt that instructs the model to return a single grouped JSON object. - Threads a new grouped: bool through the CLI, CommandType::Explain, and ExplainCommand, selecting the grouped prompt/renderer in provider::explain and ExplainCommand::execute. - --grouped conflicts with --query. Co-Authored-By: anthropic/claude-sonnet-5 --- README.md | 3 + src/ai_prompt.rs | 113 ++++++++++++ src/command/explain.rs | 51 +++++- src/command/list.rs | 1 + src/command/mod.rs | 17 +- src/config/cli.rs | 47 +++++ src/error.rs | 8 +- src/grouped_summary.rs | 395 +++++++++++++++++++++++++++++++++++++++++ src/main.rs | 8 +- src/provider/mod.rs | 5 + 10 files changed, 636 insertions(+), 12 deletions(-) create mode 100644 src/grouped_summary.rs diff --git a/README.md b/README.md index 6af76de9..b4cef9ab 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,9 @@ lumen explain main..feature/A # Ask specific questions lumen explain --query "What's the performance impact of these changes?" +# Group changes into logical clusters with a summary per group +lumen explain --grouped + # Interactive commit selection (requires: fzf) lumen explain --list ``` diff --git a/src/ai_prompt.rs b/src/ai_prompt.rs index 98ab5b2b..ebeb0e68 100644 --- a/src/ai_prompt.rs +++ b/src/ai_prompt.rs @@ -96,6 +96,64 @@ impl AIPrompt { }) } + pub fn build_grouped_explain_prompt(command: &ExplainCommand) -> Result { + let system_prompt = String::from(indoc! {r#" + You are a code-review assistant that clusters a Git diff into logical groups + based on purpose (feature, refactor, tests, config, docs, chore, etc.). + Prefer more, smaller, focused groups over fewer large ones — split unrelated + changes into separate groups even within the same purpose. + Test files must be grouped together with the implementation they test, not + split out into a separate "tests" group. + Respond with ONLY a single valid JSON object. Do not use markdown code fences. + Do not include any prose before or after the JSON object. + "#}); + + let diff_block = match &command.git_entity { + GitEntity::Commit(commit) => { + formatdoc! {" + Context - Commit: + + Message: {msg} + Changes: + ```diff + {diff} + ``` + ", + msg = commit.message, + diff = commit.diff + } + } + GitEntity::Diff(Diff::WorkingTree { diff, .. } | Diff::CommitsRange { diff, .. }) => { + formatdoc! {" + Context - Changes: + + ```diff + {diff} + ``` + " + } + } + }; + + let user_prompt = formatdoc! {r#" + {diff_block} + + Group the changes above into logical clusters and respond with ONLY a JSON object + matching this exact shape: + {{"groups":[{{"title":"string","files":["path/to/file",...],"summary":"1-3 sentence prose summary"}}],"overall_summary":"2-4 sentence prose summary, omit or use empty string if the diff is a single logical change"}} + + `files` must be exact repo-relative paths copied verbatim from the diff's file headers. + Every changed file must appear in exactly one group. If the diff represents a single + logical change, return exactly one group. + "# + }; + + Ok(AIPrompt { + system_prompt, + user_prompt, + }) + } + pub fn build_draft_prompt(command: &DraftCommand) -> Result { let GitEntity::Diff(Diff::WorkingTree { diff, .. }) = &command.git_entity else { return Err(AIPromptError( @@ -169,3 +227,58 @@ impl AIPrompt { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn working_tree_command(diff: &str, grouped: bool) -> ExplainCommand { + ExplainCommand { + git_entity: GitEntity::Diff(Diff::WorkingTree { + staged: false, + diff: diff.to_string(), + }), + query: None, + grouped, + } + } + + #[test] + fn build_explain_prompt_produces_plain_prompt_shape() { + let command = working_tree_command("diff --git a/src/a.rs b/src/a.rs", false); + + let prompt = AIPrompt::build_explain_prompt(&command).unwrap(); + + assert!(prompt + .user_prompt + .contains("diff --git a/src/a.rs b/src/a.rs")); + assert!(prompt.user_prompt.contains("Key changes")); + } + + #[test] + fn build_grouped_explain_prompt_embeds_diff_and_json_contract() { + let diff = "diff --git a/src/a.rs b/src/a.rs\n+++ b/src/a.rs"; + let command = working_tree_command(diff, true); + + let prompt = AIPrompt::build_grouped_explain_prompt(&command).unwrap(); + + assert!(prompt.user_prompt.contains(diff)); + assert!(prompt.user_prompt.contains("JSON")); + assert!(prompt.user_prompt.contains("groups")); + assert!(prompt.user_prompt.contains("overall_summary")); + } + + #[test] + fn build_grouped_explain_prompt_instructs_smaller_groups_and_colocated_tests() { + let command = working_tree_command("diff --git a/src/a.rs b/src/a.rs", true); + + let prompt = AIPrompt::build_grouped_explain_prompt(&command).unwrap(); + + assert!(prompt + .system_prompt + .contains("more, smaller, focused groups")); + assert!(prompt + .system_prompt + .contains("Test files must be grouped together with the implementation they test")); + } +} diff --git a/src/command/explain.rs b/src/command/explain.rs index 85bd68d6..3409857e 100644 --- a/src/command/explain.rs +++ b/src/command/explain.rs @@ -1,31 +1,70 @@ use spinoff::{spinners, Color, Spinner}; -use crate::{error::LumenError, git_entity::GitEntity, provider::LumenProvider}; +use crate::{ + error::LumenError, + git_entity::{diff::Diff, GitEntity}, + provider::LumenProvider, +}; use super::LumenCommand; pub struct ExplainCommand { pub git_entity: GitEntity, pub query: Option, + pub grouped: bool, +} + +/// Extract the raw diff text out of a [`GitEntity`], regardless of variant. +fn diff_text(git_entity: &GitEntity) -> &str { + match git_entity { + GitEntity::Commit(commit) => &commit.diff, + GitEntity::Diff(Diff::WorkingTree { diff, .. } | Diff::CommitsRange { diff, .. }) => diff, + } } impl ExplainCommand { + fn diff_text(&self) -> &str { + diff_text(&self.git_entity) + } + pub async fn execute(&self, provider: &LumenProvider) -> Result<(), LumenError> { LumenCommand::print_with_mdcat(self.git_entity.format_static_details(provider))?; if let Some(query) = &self.query { LumenCommand::print_with_mdcat(format!("`query`: {query}"))?; } - let spinner_text = match &self.query { - Some(_) => "Generating answer...".to_string(), - None => "Generating summary...".to_string(), + let spinner_text = if self.grouped { + "Grouping changes...".to_string() + } else { + match &self.query { + Some(_) => "Generating answer...".to_string(), + None => "Generating summary...".to_string(), + } }; let mut spinner = Spinner::new(spinners::Dots, spinner_text, Color::Blue); - let result = provider.explain(self).await?; + let result = if self.grouped { + provider.explain_grouped(self).await? + } else { + provider.explain(self).await? + }; spinner.success("Done"); - LumenCommand::print_with_mdcat(result)?; + if self.grouped { + let mut summary = crate::grouped_summary::parse_grouped_summary(&result)?; + let ground_truth = crate::grouped_summary::files_from_unified_diff(self.diff_text()); + let report = crate::grouped_summary::reconcile_groups(&mut summary, &ground_truth); + if !report.is_clean() { + eprintln!( + "note: grouping adjusted ({} unknown, {} ungrouped file(s))", + report.unknown_files.len(), + report.ungrouped_files.len() + ); + } + LumenCommand::print_with_mdcat(crate::grouped_summary::render_markdown(&summary))?; + } else { + LumenCommand::print_with_mdcat(result)?; + } Ok(()) } } diff --git a/src/command/list.rs b/src/command/list.rs index e31565c5..293a3f52 100644 --- a/src/command/list.rs +++ b/src/command/list.rs @@ -21,6 +21,7 @@ impl ListCommand { ExplainCommand { git_entity, query: None, + grouped: false, } .execute(provider) .await diff --git a/src/command/mod.rs b/src/command/mod.rs index 9bac3ad7..706ba35a 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -21,6 +21,7 @@ pub enum CommandType<'a> { Explain { git_entity: GitEntity, query: Option, + grouped: bool, }, List { backend: &'a dyn VcsBackend, @@ -46,10 +47,18 @@ impl LumenCommand { pub async fn execute(&self, command_type: CommandType<'_>) -> Result<(), LumenError> { match command_type { - CommandType::Explain { git_entity, query } => { - ExplainCommand { git_entity, query } - .execute(&self.provider) - .await + CommandType::Explain { + git_entity, + query, + grouped, + } => { + ExplainCommand { + git_entity, + query, + grouped, + } + .execute(&self.provider) + .await } CommandType::List { backend } => ListCommand.execute(&self.provider, backend).await, CommandType::Draft { diff --git a/src/config/cli.rs b/src/config/cli.rs index b34bd815..5cb6fae5 100644 --- a/src/config/cli.rs +++ b/src/config/cli.rs @@ -91,6 +91,10 @@ pub enum Commands { /// Select commit interactively using fuzzy finder #[arg(long)] list: bool, + + /// Group the changes into logical clusters with a summary per group + #[arg(long, conflicts_with = "query")] + grouped: bool, }, /// List all commits in an interactive fuzzy-finder, and summarize the changes List, @@ -182,4 +186,47 @@ mod tests { _ => panic!("expected diff command"), } } + + #[test] + fn test_explain_grouped_flag_parses() { + let cli = Cli::try_parse_from(["lumen", "explain", "--grouped"]).unwrap(); + match cli.command { + Commands::Explain { grouped, .. } => assert!(grouped), + _ => panic!("expected explain command"), + } + } + + #[test] + fn test_explain_grouped_conflicts_with_query() { + let result = Cli::try_parse_from(["lumen", "explain", "--grouped", "--query", "x"]); + assert!(result.is_err()); + } + + #[test] + fn test_explain_grouped_composes_with_staged() { + let cli = Cli::try_parse_from(["lumen", "explain", "--grouped", "--staged"]).unwrap(); + match cli.command { + Commands::Explain { + grouped, staged, .. + } => { + assert!(grouped); + assert!(staged); + } + _ => panic!("expected explain command"), + } + } + + #[test] + fn test_explain_grouped_composes_with_reference() { + let cli = Cli::try_parse_from(["lumen", "explain", "--grouped", "HEAD"]).unwrap(); + match cli.command { + Commands::Explain { + grouped, reference, .. + } => { + assert!(grouped); + assert!(reference.is_some()); + } + _ => panic!("expected explain command"), + } + } } diff --git a/src/error.rs b/src/error.rs index 5d9d8371..da595a31 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,7 @@ -use crate::{git_entity::diff::DiffError, provider::ProviderError, vcs::VcsError}; +use crate::{ + git_entity::diff::DiffError, grouped_summary::GroupedSummaryError, provider::ProviderError, + vcs::VcsError, +}; use std::io; use thiserror::Error; @@ -34,4 +37,7 @@ pub enum LumenError { #[error("JSON error: {0}")] JsonError(#[from] serde_json::Error), + + #[error(transparent)] + GroupedSummaryError(#[from] GroupedSummaryError), } diff --git a/src/grouped_summary.rs b/src/grouped_summary.rs new file mode 100644 index 00000000..b9bfbde6 --- /dev/null +++ b/src/grouped_summary.rs @@ -0,0 +1,395 @@ +use std::collections::HashSet; + +use serde::Deserialize; +use thiserror::Error; + +#[derive(Debug, Clone, Deserialize)] +pub struct GroupedSummary { + pub groups: Vec, + #[serde(default)] + pub overall_summary: Option, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct DiffGroup { + pub title: String, + #[serde(default)] + pub files: Vec, + pub summary: String, +} + +#[derive(Error, Debug)] +pub enum GroupedSummaryError { + #[error("could not parse grouped summary as JSON: {0}")] + Parse(#[from] serde_json::Error), + #[error("model returned an empty grouped summary")] + Empty, +} + +#[derive(Debug, Default)] +pub struct ReconcileReport { + /// Referenced by a group but not in ground truth. + pub unknown_files: Vec, + /// In ground truth but appear in no group. + pub ungrouped_files: Vec, +} + +impl ReconcileReport { + pub fn is_clean(&self) -> bool { + self.unknown_files.is_empty() && self.ungrouped_files.is_empty() + } +} + +/// Strip a markdown code fence around a JSON payload, with a fallback that +/// slices out the outermost `{...}` if stray prose surrounds the JSON. +fn strip_json_fence(raw: &str) -> &str { + let trimmed = raw.trim(); + + let unfenced = if trimmed.starts_with("```") { + let without_leading_fence = trimmed + .strip_prefix("```") + .and_then(|rest| rest.split_once('\n')) + .map(|(_, rest)| rest) + .unwrap_or(trimmed); + + without_leading_fence + .trim_end() + .strip_suffix("```") + .unwrap_or(without_leading_fence) + .trim() + } else { + trimmed + }; + + if unfenced.starts_with('{') { + return unfenced; + } + + match (unfenced.find('{'), unfenced.rfind('}')) { + (Some(start), Some(end)) if start <= end => &unfenced[start..=end], + _ => trimmed, + } +} + +/// Parse a model response into a [`GroupedSummary`], tolerating markdown +/// fences and stray prose around the JSON payload. +pub fn parse_grouped_summary(raw: &str) -> Result { + let json = strip_json_fence(raw); + let summary: GroupedSummary = serde_json::from_str(json)?; + + if summary.groups.is_empty() { + return Err(GroupedSummaryError::Empty); + } + + Ok(summary) +} + +/// Extract the ordered, deduplicated list of file paths changed in a unified +/// diff, straight from its `+++`/`---` file headers. +pub fn files_from_unified_diff(diff: &str) -> Vec { + let mut seen = HashSet::new(); + let mut files = Vec::new(); + + let lines: Vec<&str> = diff.lines().collect(); + for (idx, line) in lines.iter().enumerate() { + let path = if let Some(new_path) = line.strip_prefix("+++ b/") { + Some(new_path.to_string()) + } else if *line == "+++ /dev/null" { + lines + .get(idx.wrapping_sub(1)) + .and_then(|prev| prev.strip_prefix("--- a/")) + .map(str::to_string) + } else { + None + }; + + if let Some(path) = path { + if seen.insert(path.clone()) { + files.push(path); + } + } + } + + files +} + +/// Cross-check the model's grouping against the diff's ground-truth file +/// list, dropping hallucinated files and collecting anything left out into +/// an `Ungrouped` catch-all group. +pub fn reconcile_groups(summary: &mut GroupedSummary, ground_truth: &[String]) -> ReconcileReport { + let known: HashSet<&str> = ground_truth.iter().map(String::as_str).collect(); + let mut report = ReconcileReport::default(); + let mut seen_unknown = HashSet::new(); + let mut grouped_files: HashSet = HashSet::new(); + + for group in &mut summary.groups { + let mut kept = Vec::with_capacity(group.files.len()); + for file in group.files.drain(..) { + if known.contains(file.as_str()) { + grouped_files.insert(file.clone()); + kept.push(file); + } else if seen_unknown.insert(file.clone()) { + report.unknown_files.push(file); + } + } + group.files = kept; + } + + let ungrouped: Vec = ground_truth + .iter() + .filter(|file| !grouped_files.contains(file.as_str())) + .cloned() + .collect(); + + if !ungrouped.is_empty() { + report.ungrouped_files = ungrouped.clone(); + summary.groups.push(DiffGroup { + title: "Ungrouped".to_string(), + files: ungrouped, + summary: "Files not confidently assigned to a group.".to_string(), + }); + } + + report +} + +/// Render a [`GroupedSummary`] as markdown for terminal display. +pub fn render_markdown(summary: &GroupedSummary) -> String { + let mut out = String::new(); + + for group in &summary.groups { + out.push_str(&format!("## {}\n\n", group.title)); + for file in &group.files { + out.push_str(&format!("- `{}`\n", file)); + } + out.push('\n'); + out.push_str(&group.summary); + out.push_str("\n\n"); + } + + if let Some(overall) = &summary.overall_summary { + if !overall.is_empty() { + out.push_str("## Overall Summary\n\n"); + out.push_str(overall); + out.push('\n'); + } + } + + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strip_json_fence_passes_through_plain_json() { + let raw = r#"{"groups":[]}"#; + assert_eq!(strip_json_fence(raw), r#"{"groups":[]}"#); + } + + #[test] + fn strip_json_fence_strips_json_language_tag() { + let raw = "```json\n{\"groups\":[]}\n```"; + assert_eq!(strip_json_fence(raw), "{\"groups\":[]}"); + } + + #[test] + fn strip_json_fence_strips_bare_fence() { + let raw = "```\n{\"groups\":[]}\n```"; + assert_eq!(strip_json_fence(raw), "{\"groups\":[]}"); + } + + #[test] + fn strip_json_fence_extracts_json_from_stray_prose() { + let raw = "Here you go:\n{\"groups\":[]}\nHope that helps!"; + assert_eq!(strip_json_fence(raw), "{\"groups\":[]}"); + } + + #[test] + fn parse_grouped_summary_parses_groups_and_overall_summary() { + let raw = r#"{"groups":[{"title":"Feature","files":["src/a.rs"],"summary":"Adds a."}],"overall_summary":"Adds feature a."}"#; + let summary = parse_grouped_summary(raw).unwrap(); + assert_eq!(summary.groups.len(), 1); + assert_eq!(summary.groups[0].title, "Feature"); + assert_eq!(summary.overall_summary, Some("Adds feature a.".to_string())); + } + + #[test] + fn parse_grouped_summary_defaults_overall_summary_to_none() { + let raw = r#"{"groups":[{"title":"Feature","files":["src/a.rs"],"summary":"Adds a."}]}"#; + let summary = parse_grouped_summary(raw).unwrap(); + assert_eq!(summary.overall_summary, None); + } + + #[test] + fn parse_grouped_summary_rejects_empty_groups() { + let raw = r#"{"groups":[]}"#; + assert!(matches!( + parse_grouped_summary(raw), + Err(GroupedSummaryError::Empty) + )); + } + + #[test] + fn parse_grouped_summary_rejects_garbage() { + let raw = "not json at all"; + assert!(matches!( + parse_grouped_summary(raw), + Err(GroupedSummaryError::Parse(_)) + )); + } + + #[test] + fn files_from_unified_diff_returns_ordered_deduped_paths() { + let diff = indoc::indoc! {" + diff --git a/src/a.rs b/src/a.rs + index 1111111..2222222 100644 + --- a/src/a.rs + +++ b/src/a.rs + @@ -1,1 +1,1 @@ + -old + +new + diff --git a/src/b.rs b/src/b.rs + index 3333333..4444444 100644 + --- a/src/b.rs + +++ b/src/b.rs + @@ -1,1 +1,1 @@ + -old + +new + "}; + + assert_eq!( + files_from_unified_diff(diff), + vec!["src/a.rs".to_string(), "src/b.rs".to_string()] + ); + } + + #[test] + fn files_from_unified_diff_falls_back_to_old_path_for_deletions() { + let diff = indoc::indoc! {" + diff --git a/src/deleted.rs b/src/deleted.rs + deleted file mode 100644 + index 1111111..0000000 + --- a/src/deleted.rs + +++ /dev/null + @@ -1,1 +0,0 @@ + -gone + "}; + + assert_eq!( + files_from_unified_diff(diff), + vec!["src/deleted.rs".to_string()] + ); + } + + #[test] + fn reconcile_groups_is_clean_when_all_files_accounted_for() { + let mut summary = GroupedSummary { + groups: vec![DiffGroup { + title: "Feature".to_string(), + files: vec!["src/a.rs".to_string()], + summary: "Adds a.".to_string(), + }], + overall_summary: None, + }; + let ground_truth = vec!["src/a.rs".to_string()]; + + let report = reconcile_groups(&mut summary, &ground_truth); + + assert!(report.is_clean()); + assert_eq!(summary.groups.len(), 1); + } + + #[test] + fn reconcile_groups_drops_unknown_files_and_reports_them() { + let mut summary = GroupedSummary { + groups: vec![DiffGroup { + title: "Feature".to_string(), + files: vec!["src/a.rs".to_string(), "src/hallucinated.rs".to_string()], + summary: "Adds a.".to_string(), + }], + overall_summary: None, + }; + let ground_truth = vec!["src/a.rs".to_string()]; + + let report = reconcile_groups(&mut summary, &ground_truth); + + assert_eq!(summary.groups[0].files, vec!["src/a.rs".to_string()]); + assert_eq!( + report.unknown_files, + vec!["src/hallucinated.rs".to_string()] + ); + assert!(!report.is_clean()); + } + + #[test] + fn reconcile_groups_appends_ungrouped_group_for_missing_files() { + let mut summary = GroupedSummary { + groups: vec![DiffGroup { + title: "Feature".to_string(), + files: vec!["src/a.rs".to_string()], + summary: "Adds a.".to_string(), + }], + overall_summary: None, + }; + let ground_truth = vec!["src/a.rs".to_string(), "src/b.rs".to_string()]; + + let report = reconcile_groups(&mut summary, &ground_truth); + + assert_eq!(report.ungrouped_files, vec!["src/b.rs".to_string()]); + assert!(!report.is_clean()); + assert_eq!(summary.groups.len(), 2); + let ungrouped = &summary.groups[1]; + assert_eq!(ungrouped.title, "Ungrouped"); + assert_eq!(ungrouped.files, vec!["src/b.rs".to_string()]); + } + + #[test] + fn render_markdown_renders_groups_and_overall_summary() { + let summary = GroupedSummary { + groups: vec![ + DiffGroup { + title: "Feature".to_string(), + files: vec!["src/a.rs".to_string()], + summary: "Adds a.".to_string(), + }, + DiffGroup { + title: "Tests".to_string(), + files: vec!["tests/a_test.rs".to_string()], + summary: "Covers a.".to_string(), + }, + ], + overall_summary: Some("Adds feature a with tests.".to_string()), + }; + + let expected = "## Feature\n\n\ + - `src/a.rs`\n\ + \n\ + Adds a.\n\n\ + ## Tests\n\n\ + - `tests/a_test.rs`\n\ + \n\ + Covers a.\n\n\ + ## Overall Summary\n\n\ + Adds feature a with tests.\n"; + + assert_eq!(render_markdown(&summary), expected); + } + + #[test] + fn render_markdown_omits_overall_summary_section_when_none() { + let summary = GroupedSummary { + groups: vec![DiffGroup { + title: "Feature".to_string(), + files: vec!["src/a.rs".to_string()], + summary: "Adds a.".to_string(), + }], + overall_summary: None, + }; + + let expected = "## Feature\n\n- `src/a.rs`\n\nAdds a.\n\n"; + + assert_eq!(render_markdown(&summary), expected); + } +} diff --git a/src/main.rs b/src/main.rs index b335bf5e..5aeeaab9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ mod commit_reference; mod config; mod error; mod git_entity; +mod grouped_summary; mod provider; mod vcs; @@ -48,6 +49,7 @@ async fn run() -> Result<(), LumenError> { staged, query, list, + grouped, } => { let git_entity = if list { let sha = LumenCommand::get_sha_from_fzf(backend.as_ref())?; @@ -94,7 +96,11 @@ async fn run() -> Result<(), LumenError> { }; command - .execute(command::CommandType::Explain { git_entity, query }) + .execute(command::CommandType::Explain { + git_entity, + query, + grouped, + }) .await?; } Commands::List => { diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 5057a0e9..483c27b6 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -158,6 +158,11 @@ impl LumenProvider { self.complete(prompt).await } + pub async fn explain_grouped(&self, command: &ExplainCommand) -> Result { + let prompt = AIPrompt::build_grouped_explain_prompt(command)?; + self.complete(prompt).await + } + pub async fn draft(&self, command: &DraftCommand) -> Result { let prompt = AIPrompt::build_draft_prompt(command)?; self.complete(prompt).await From 9c656e446f215ee335424e2f548565b70d60b8d8 Mon Sep 17 00:00:00 2001 From: Andrew Thal <467872+athal7@users.noreply.github.com> Date: Thu, 9 Jul 2026 13:22:24 -0500 Subject: [PATCH 2/3] feat(diff): add AI-grouped change clustering with group-level viewed marking Adds an 'a' keybinding in the diff TUI that asks the AI to cluster the current diff into logical groups with a per-group summary, shown in a new scrollable modal, and lets the group's files be marked viewed together (reusing the existing viewed_files/GitHub-sync machinery). - Thread the provider through as Arc (main.rs, command/mod.rs, command/diff/app.rs) so the diff TUI can share it with a background grouping request. - Add combined_unified_diff() to concatenate non-binary FileDiffs into one unified diff string for the grouping prompt. - Add generate_groups_async() to request a grouped summary via ExplainCommand{grouped: true} on a background OS thread and report the reconciled result back over an mpsc channel. Takes a pre-captured tokio::runtime::Handle rather than calling Handle::current() inside the spawned thread, since a bare std::thread::spawn closure has no ambient runtime context and that call would panic. - New DiffGroups modal (title, files, and prose summary per group, scrollable), cached on AppState.diff_groups and invalidated on reload. - Space inside the modal marks all files in the selected group as viewed (set-only, syncs to GitHub in PR mode) without dismissing the modal, so multiple groups can be marked in one sitting. - Add AppState::file_index_for_path and the standalone mark_paths_viewed helper backing the group-viewed action. - Document the new 'a' keybinding in the help modal. Co-Authored-By: anthropic/claude-sonnet-5 --- src/command/diff/app.rs | 318 ++++++++++++++++++++++++--- src/command/diff/git.rs | 2 + src/command/diff/mod.rs | 211 +++++++++++++++++- src/command/diff/render/diff_view.rs | 88 +++++--- src/command/diff/render/mod.rs | 1 + src/command/diff/render/modal.rs | 209 ++++++++++++------ src/command/diff/render/sidebar.rs | 304 ++++++++++++++++++++----- src/command/diff/state.rs | 186 +++++++++++++++- src/command/mod.rs | 5 +- src/config/cli.rs | 13 ++ src/config/configuration.rs | 5 + src/main.rs | 13 +- 12 files changed, 1149 insertions(+), 206 deletions(-) diff --git a/src/command/diff/app.rs b/src/command/diff/app.rs index f60736ee..0f5ef11c 100644 --- a/src/command/diff/app.rs +++ b/src/command/diff/app.rs @@ -42,10 +42,13 @@ use super::git::{ }; use super::highlight; use super::render::{ - render_diff, render_empty_state, truncate_path, FilePickerItem, KeyBind, KeyBindSection, Modal, - ModalContent, ModalFileStatus, ModalResult, + render_diff, render_empty_state, truncate_path, FilePickerItem, GuideStatus, KeyBind, + KeyBindSection, Modal, ModalContent, ModalFileStatus, ModalResult, +}; +use super::state::{ + adjust_scroll_for_hunk, adjust_scroll_to_line, mark_paths_viewed, AppState, PendingKey, + SidebarMode, }; -use super::state::{adjust_scroll_for_hunk, adjust_scroll_to_line, AppState, PendingKey}; use super::theme; use super::types::{ ChangeType, CursorPosition, DiffFullscreen, DiffPanelFocus, FileStatus, FocusedPanel, @@ -53,7 +56,8 @@ use super::types::{ }; use super::watcher::{setup_watcher, WatchEvent}; use super::{ - fetch_viewed_files, mark_file_as_viewed_async, unmark_file_as_viewed_async, DiffOptions, PrInfo, + combined_unified_diff, fetch_viewed_files, mark_file_as_viewed_async, + unmark_file_as_viewed_async, DiffOptions, GenerateRequest, GroupResult, PrInfo, }; use spinoff::{spinners, Color, Spinner}; @@ -67,6 +71,7 @@ fn navigate_stacked_commit( new_index: usize, options: &DiffOptions, backend: &dyn VcsBackend, + req_tx: &std::sync::mpsc::Sender, ) -> bool { if new_index >= state.stacked_commits.len() { return false; @@ -77,12 +82,94 @@ fn navigate_stacked_commit( let file_diffs = load_single_commit_diffs(&commit.commit_id, &options.file, backend); state.reload(file_diffs, None); state.load_stacked_viewed_files(); + state.guide_group_selected = 0; + state.guide_file_selected = 0; + maybe_trigger_group_generation(state, options, req_tx); true } else { false } } +/// Identity of the diff currently displayed, used to key the grouped-summary +/// cache. Stacked mode identifies by commit SHA (stable across content +/// changes to the same commit during a session); every other mode hashes +/// the combined unified diff, so any content change produces a new key. +fn current_diff_identity(state: &AppState) -> String { + if state.stacked_mode { + return state + .current_commit() + .map(|c| c.commit_id.clone()) + .unwrap_or_default(); + } + use sha2::{Digest, Sha256}; + let combined = combined_unified_diff(&state.file_diffs); + let mut hasher = Sha256::new(); + hasher.update(combined.as_bytes()); + format!("{:x}", hasher.finalize()) +} + +/// Kick off a grouped-summary request for the current diff if the Guide +/// feature is enabled and no cached/in-flight result already covers it. +/// Always refreshes `state.current_diff_identity` first, even when +/// `--guide` is off, so other code can rely on it being current. +fn maybe_trigger_group_generation( + state: &mut AppState, + options: &DiffOptions, + req_tx: &std::sync::mpsc::Sender, +) { + if state.file_diffs.is_empty() { + return; + } + let identity = current_diff_identity(state); + state.current_diff_identity = identity.clone(); + if !options.guide { + return; + } + if state.groups_cache.contains_key(&identity) || state.groups_pending.contains(&identity) { + return; + } + let combined = combined_unified_diff(&state.file_diffs); + let ground_truth: Vec = state + .file_diffs + .iter() + .map(|f| f.filename.clone()) + .collect(); + state.groups_pending.insert(identity.clone()); + let _ = req_tx.send(GenerateRequest { + identity, + combined_diff: combined, + ground_truth, + }); +} + +/// Files in the group currently selected in Guide mode, if the grouped +/// summary for the active diff has already been generated. +fn current_guide_group_files(state: &AppState) -> Option<&Vec> { + state + .groups_cache + .get(&state.current_diff_identity)? + .groups + .get(state.guide_group_selected) + .map(|g| &g.files) +} + +/// Switch the diff view to the file at `index_in_group` within the +/// currently selected Guide-mode group, using the same selection mechanism +/// as jumping to a file from the picker or an annotation +/// (`AppState::select_file`). No-op if the group/file no longer resolves to +/// a known path. +fn select_guide_file(state: &mut AppState, index_in_group: usize) { + let path = current_guide_group_files(state) + .and_then(|files| files.get(index_in_group)) + .cloned(); + if let Some(path) = path { + if let Some(idx) = state.file_index_for_path(&path) { + state.select_file(idx); + } + } +} + /// Adjust sidebar scroll to ensure the selected item is visible. fn ensure_sidebar_visible(state: &mut AppState, visible_height: usize) { if state.sidebar_selected >= state.sidebar_scroll + visible_height { @@ -254,9 +341,19 @@ pub fn run_app_with_pr( options: DiffOptions, pr_info: PrInfo, backend: &dyn VcsBackend, + req_tx: std::sync::mpsc::Sender, + res_rx: std::sync::mpsc::Receiver, ) -> io::Result<()> { match load_pr_file_diffs(&pr_info) { - Ok(file_diffs) => run_app_internal(options, Some(pr_info), file_diffs, None, backend), + Ok(file_diffs) => run_app_internal( + options, + Some(pr_info), + file_diffs, + None, + backend, + req_tx, + res_rx, + ), Err(_) => std::process::exit(1), } } @@ -265,20 +362,32 @@ pub fn run_app( options: DiffOptions, pr_info: Option, backend: &dyn VcsBackend, + req_tx: std::sync::mpsc::Sender, + res_rx: std::sync::mpsc::Receiver, ) -> io::Result<()> { let file_diffs = load_file_diffs(&options, backend); - run_app_internal(options, pr_info, file_diffs, None, backend) + run_app_internal(options, pr_info, file_diffs, None, backend, req_tx, res_rx) } pub fn run_app_stacked( options: DiffOptions, commits: Vec, backend: &dyn VcsBackend, + req_tx: std::sync::mpsc::Sender, + res_rx: std::sync::mpsc::Receiver, ) -> io::Result<()> { // Load the first commit's diff let first_commit = &commits[0]; let file_diffs = load_single_commit_diffs(&first_commit.commit_id, &options.file, backend); - run_app_internal(options, None, file_diffs, Some(commits), backend) + run_app_internal( + options, + None, + file_diffs, + Some(commits), + backend, + req_tx, + res_rx, + ) } /// Sync viewed files from GitHub to local state @@ -299,6 +408,8 @@ fn run_app_internal( file_diffs: Vec, stacked_commits: Option>, backend: &dyn VcsBackend, + req_tx: std::sync::mpsc::Sender, + res_rx: std::sync::mpsc::Receiver, ) -> io::Result<()> { theme::init(options.theme.as_deref()); highlight::init(); @@ -375,6 +486,8 @@ fn run_app_internal( let mut pending_events: VecDeque = VecDeque::new(); let mut send_annotations_on_exit = false; + maybe_trigger_group_generation(&mut state, &options, &req_tx); + 'main: loop { if let Some(ref rx) = watch_rx { match rx.try_recv() { @@ -387,6 +500,19 @@ fn run_app_internal( } } + match res_rx.try_recv() { + Ok((id, Ok(summary))) => { + state.groups_pending.remove(&id); + state.groups_errors.remove(&id); + state.groups_cache.insert(id, summary); + } + Ok((id, Err(e))) => { + state.groups_pending.remove(&id); + state.groups_errors.insert(id, e); + } + Err(_) => {} + } + if state.needs_reload { let file_diffs = if let Some(ref pr) = pr_info { // In PR mode, reload from GitHub @@ -409,6 +535,8 @@ fn run_app_internal( if let Some(ref pr) = pr_info { sync_viewed_files_from_github(pr, &mut state); } + + maybe_trigger_group_generation(&mut state, &options, &req_tx); } if state.file_diffs.is_empty() { @@ -436,10 +564,31 @@ fn run_app_internal( .unwrap_or(&empty_viewed_hunks); let branch_fallback = get_current_branch(backend); let commit_ref = state.diff_reference.as_deref().unwrap_or(&branch_fallback); + let guide_groups: &[crate::grouped_summary::DiffGroup] = state + .groups_cache + .get(&state.current_diff_identity) + .map(|s| s.groups.as_slice()) + .unwrap_or(&[]); + let guide_status = if !options.guide { + GuideStatus::Disabled + } else if let Some(summary) = state.groups_cache.get(&state.current_diff_identity) { + if summary.groups.is_empty() { + GuideStatus::Empty + } else { + GuideStatus::Ready + } + } else if state.groups_pending.contains(&state.current_diff_identity) { + GuideStatus::Pending + } else if let Some(e) = state.groups_errors.get(&state.current_diff_identity) { + GuideStatus::Error(e.clone()) + } else { + GuideStatus::Empty + }; let row_offset = std::cell::Cell::new(0usize); let gaps_cell = std::cell::RefCell::new(Vec::new()); let rects_cell = std::cell::RefCell::new(Vec::new()); - let editor_rect_cell: std::cell::Cell> = std::cell::Cell::new(None); + let editor_rect_cell: std::cell::Cell> = + std::cell::Cell::new(None); terminal.draw(|frame| { let (offset, gaps, rects, er) = render_diff( frame, @@ -485,6 +634,11 @@ fn run_app_internal( state.total_added, state.total_removed, annotation_editor.as_ref(), + state.sidebar_mode, + guide_groups, + state.guide_group_selected, + state.guide_file_selected, + guide_status.clone(), ); row_offset.set(offset); *rects_cell.borrow_mut() = rects; @@ -950,7 +1104,7 @@ fn run_app_internal( if mouse.column < 4 && state.current_commit_index > 0 { let new_index = state.current_commit_index - 1; navigate_stacked_commit( - &mut state, new_index, &options, backend, + &mut state, new_index, &options, backend, &req_tx, ); } // Right arrow click (last 4 columns to cover " > ") @@ -960,7 +1114,7 @@ fn run_app_internal( { let new_index = state.current_commit_index + 1; navigate_stacked_commit( - &mut state, new_index, &options, backend, + &mut state, new_index, &options, backend, &req_tx, ); } } else if state.show_sidebar @@ -1330,14 +1484,18 @@ fn run_app_internal( && state.current_commit_index < state.stacked_commits.len() - 1 { let new_index = state.current_commit_index + 1; - navigate_stacked_commit(&mut state, new_index, &options, backend); + navigate_stacked_commit( + &mut state, new_index, &options, backend, &req_tx, + ); } } // Stacked mode: navigate to previous commit KeyCode::Char('h') if key.modifiers.contains(KeyModifiers::CONTROL) => { if state.stacked_mode && state.current_commit_index > 0 { let new_index = state.current_commit_index - 1; - navigate_stacked_commit(&mut state, new_index, &options, backend); + navigate_stacked_commit( + &mut state, new_index, &options, backend, &req_tx, + ); } } KeyCode::Char('d') if key.modifiers.contains(KeyModifiers::CONTROL) => { @@ -1426,7 +1584,17 @@ fn run_app_internal( } } KeyCode::Down | KeyCode::Char('j') => { - if state.focused_panel == FocusedPanel::Sidebar { + if state.sidebar_mode == SidebarMode::Guide { + if let Some(len) = + current_guide_group_files(&state).map(|f| f.len()) + { + if len > 0 && state.guide_file_selected + 1 < len { + state.guide_file_selected += 1; + let next = state.guide_file_selected; + select_guide_file(&mut state, next); + } + } + } else if state.focused_panel == FocusedPanel::Sidebar { if state.sidebar_selected + 1 < state.sidebar_visible_len() { state.sidebar_selected += 1; } @@ -1438,7 +1606,13 @@ fn run_app_internal( } } KeyCode::Up | KeyCode::Char('k') => { - if state.focused_panel == FocusedPanel::Sidebar { + if state.sidebar_mode == SidebarMode::Guide { + if state.guide_file_selected > 0 { + state.guide_file_selected -= 1; + let next = state.guide_file_selected; + select_guide_file(&mut state, next); + } + } else if state.focused_panel == FocusedPanel::Sidebar { if state.sidebar_selected > 0 { state.sidebar_selected = state.sidebar_selected.saturating_sub(1); @@ -1518,7 +1692,36 @@ fn run_app_internal( } } KeyCode::Char(' ') => { - if state.focused_panel == FocusedPanel::Sidebar + if state.sidebar_mode == SidebarMode::Guide { + if let Some(files) = current_guide_group_files(&state).cloned() { + if let Some(path) = files.get(state.guide_file_selected) { + if let Some(file_idx) = state.file_index_for_path(path) { + let filename = + state.file_diffs[file_idx].filename.clone(); + let was_viewed = state.viewed_files.contains(&file_idx); + + // Optimistic update - update local state immediately + if was_viewed { + state.viewed_files.remove(&file_idx); + } else { + state.viewed_files.insert(file_idx); + } + + // Fire off async API call if in PR mode + if let Some(ref pr) = pr_info { + if was_viewed { + unmark_file_as_viewed_async(pr, &filename); + } else { + mark_file_as_viewed_async(pr, &filename); + } + } + } + } + if state.guide_file_selected + 1 < files.len() { + state.guide_file_selected += 1; + } + } + } else if state.focused_panel == FocusedPanel::Sidebar && state.sidebar_selected < state.sidebar_visible_len() { let selected = state @@ -1746,6 +1949,52 @@ fn run_app_internal( } } } + KeyCode::Char('a') => { + // Nothing to show without --guide: leave the sidebar alone. + if options.guide { + state.sidebar_mode = match state.sidebar_mode { + SidebarMode::Directory => SidebarMode::Guide, + SidebarMode::Guide => SidebarMode::Directory, + }; + if state.sidebar_mode == SidebarMode::Guide { + state.guide_group_selected = 0; + state.guide_file_selected = 0; + state.focused_panel = FocusedPanel::Sidebar; + select_guide_file(&mut state, 0); + } + } + } + KeyCode::Char(',') => { + if state.sidebar_mode == SidebarMode::Guide + && state.guide_group_selected > 0 + { + state.guide_group_selected -= 1; + state.guide_file_selected = 0; + select_guide_file(&mut state, 0); + } + } + KeyCode::Char('.') => { + if state.sidebar_mode == SidebarMode::Guide { + if let Some(len) = state + .groups_cache + .get(&state.current_diff_identity) + .map(|s| s.groups.len()) + { + if state.guide_group_selected + 1 < len { + state.guide_group_selected += 1; + state.guide_file_selected = 0; + select_guide_file(&mut state, 0); + } + } + } + } + KeyCode::Char('V') => { + if state.sidebar_mode == SidebarMode::Guide { + if let Some(files) = current_guide_group_files(&state).cloned() { + mark_paths_viewed(&mut state, &files, pr_info.as_ref()); + } + } + } KeyCode::Char('i') => { if !state.file_diffs.is_empty() { let file_index = state.current_file; @@ -1900,10 +2149,8 @@ fn run_app_internal( } KeyCode::Char('e') => { if !state.file_diffs.is_empty() { - let _ = execute!( - terminal.backend_mut(), - PopKeyboardEnhancementFlags - ); + let _ = + execute!(terminal.backend_mut(), PopKeyboardEnhancementFlags); execute!( terminal.backend_mut(), DisableMouseCapture, @@ -1952,7 +2199,9 @@ fn run_app_internal( )?; let _ = execute!( terminal.backend_mut(), - PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES) + PushKeyboardEnhancementFlags( + KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES + ) ); terminal.clear()?; } @@ -2106,10 +2355,10 @@ fn run_app_internal( key: "h/l or left/right", description: "Scroll horizontally", }, - KeyBind { - key: "w", - description: "Toggle watch mode", - }, + KeyBind { + key: "w", + description: "Toggle watch mode", + }, KeyBind { key: "gg / G", description: "Scroll to top / bottom", @@ -2153,7 +2402,8 @@ fn run_app_internal( }, KeyBind { key: "ctrl+f", - description: "Global fuzzy search (all files, with preview)", + description: + "Global fuzzy search (all files, with preview)", }, KeyBind { key: "n or down", @@ -2190,6 +2440,24 @@ fn run_app_internal( }, ], }, + KeyBindSection { + title: "AI Guide (--guide)", + bindings: vec![ + KeyBind { + key: "a", + description: "Toggle AI guide view", + }, + KeyBind { + key: ", / .", + description: "Guide: previous / next group", + }, + KeyBind { + key: "V", + description: + "Guide: mark all files in group viewed", + }, + ], + }, ], )); } diff --git a/src/command/diff/git.rs b/src/command/diff/git.rs index 44ec6286..40571b74 100644 --- a/src/command/diff/git.rs +++ b/src/command/diff/git.rs @@ -537,6 +537,7 @@ mod tests { focus: None, origin: None, wrap: false, + guide: false, }; let diffs = load_file_diffs(&options, &backend); @@ -609,6 +610,7 @@ mod tests { focus: None, origin: None, wrap: false, + guide: false, }; let diffs = load_file_diffs(&options, &backend); diff --git a/src/command/diff/mod.rs b/src/command/diff/mod.rs index c1ce8199..f8f332b4 100644 --- a/src/command/diff/mod.rs +++ b/src/command/diff/mod.rs @@ -18,11 +18,13 @@ mod watcher; use std::collections::HashSet; use std::io; use std::process::{self, Command}; +use std::sync::Arc; use std::thread; use spinoff::{spinners, Color, Spinner}; use crate::commit_reference::CommitReference; +use crate::provider::LumenProvider; use crate::vcs::VcsBackend; pub struct DiffOptions { @@ -36,6 +38,7 @@ pub struct DiffOptions { pub focus: Option, pub origin: Option, pub wrap: bool, + pub guide: bool, } #[derive(Clone)] @@ -100,7 +103,10 @@ fn resolve_origin_repo() -> Result { if parts.len() >= 2 { Ok(format!("{}/{}", parts[0], parts[1])) } else { - Err(format!("Could not parse owner/repo from origin URL: {}", url)) + Err(format!( + "Could not parse owner/repo from origin URL: {}", + url + )) } } @@ -298,6 +304,108 @@ fn mark_file_as_viewed_sync(node_id: &str, file_path: &str) -> Result<(), String Ok(()) } +/// Concatenate the non-binary files in `file_diffs` into a single unified +/// diff, as if `git diff` had produced them all in one invocation. Used to +/// feed the whole working-tree diff to the model for grouped summarization. +pub fn combined_unified_diff(file_diffs: &[types::FileDiff]) -> String { + let mut combined = String::new(); + + for file_diff in file_diffs { + if file_diff.is_binary { + continue; + } + + combined.push_str(&format!( + "diff --git a/{path} b/{path}\n", + path = file_diff.filename + )); + + let a_path = format!("a/{}", file_diff.filename); + let b_path = format!("b/{}", file_diff.filename); + let body = similar::TextDiff::from_lines(&file_diff.old_content, &file_diff.new_content) + .unified_diff() + .header(&a_path, &b_path) + .to_string(); + combined.push_str(&body); + } + + combined +} + +/// A single grouped-summary request: the diff identity it's keyed against +/// (see `current_diff_identity` in `app.rs`), the combined diff text to send +/// to the model, and the ground-truth file list used to reconcile the +/// model's grouping against what actually changed. +pub struct GenerateRequest { + pub identity: String, + pub combined_diff: String, + pub ground_truth: Vec, +} + +/// Result of a `GenerateRequest`, tagged with the identity it was requested +/// for so the receiver can route it back to the right cache entry even if +/// the user has since navigated to a different diff. +pub type GroupResult = ( + String, + Result, +); + +/// Spawn the single background grouping worker for the lifetime of the TUI +/// session. Owns `provider` and a Tokio `handle` captured on a runtime +/// thread, loops on `req_rx` until the sender side is dropped (TUI exit), +/// and reports `(identity, result)` back over `res_tx` for each request in +/// turn. Requests are processed serially — the previous per-keypress thread +/// spawning is replaced by this persistent worker so overlapping requests +/// can't race each other. +/// +/// `handle` must be captured on a thread that is already inside a Tokio +/// runtime context (e.g. via `tokio::runtime::Handle::current()` from the +/// synchronous TUI loop, which itself runs on a Tokio worker thread). A +/// plain `std::thread::spawn` closure has no ambient runtime, so calling +/// `Handle::current()` *inside* the spawned thread panics with "there is no +/// reactor running" — the handle must be captured outside and moved in. +pub fn spawn_group_worker( + provider: Arc, + handle: tokio::runtime::Handle, + req_rx: std::sync::mpsc::Receiver, + res_tx: std::sync::mpsc::Sender, +) { + thread::spawn(move || { + for req in req_rx { + let GenerateRequest { + identity, + combined_diff, + ground_truth, + } = req; + let result = handle.block_on(async { + let cmd = crate::command::explain::ExplainCommand { + git_entity: crate::git_entity::GitEntity::Diff( + crate::git_entity::diff::Diff::WorkingTree { + staged: false, + diff: combined_diff, + }, + ), + query: None, + grouped: true, + }; + provider.explain_grouped(&cmd).await + }); + + let parsed = result + .map_err(|e| e.to_string()) + .and_then(|raw| { + crate::grouped_summary::parse_grouped_summary(&raw).map_err(|e| e.to_string()) + }) + .map(|mut summary| { + crate::grouped_summary::reconcile_groups(&mut summary, &ground_truth); + summary + }); + + let _ = res_tx.send((identity, parsed)); + } + }); +} + /// Unmark a file as viewed on GitHub PR (blocking) fn unmark_file_as_viewed_sync(node_id: &str, file_path: &str) -> Result<(), String> { let mutation = format!( @@ -338,7 +446,20 @@ fn detect_current_branch_pr() -> Result { Ok(number) } -pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Result<()> { +pub fn run_diff_ui( + mut options: DiffOptions, + backend: &dyn VcsBackend, + provider: Arc, +) -> io::Result<()> { + // Spawn the single grouping worker up front and hand every dispatch + // branch below its own request-sender clone plus the (single) result + // receiver. `provider` is moved into the worker here — it's no longer + // threaded down into `app::run_app*`. + let handle = tokio::runtime::Handle::current(); + let (req_tx, req_rx) = std::sync::mpsc::channel::(); + let (res_tx, res_rx) = std::sync::mpsc::channel::(); + spawn_group_worker(provider, handle, req_rx, res_tx); + // Resolve --detect-pr into options.pr if options.detect_pr && options.pr.is_none() { let mut spinner = Spinner::new( @@ -373,7 +494,7 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re match fetch_pr_info(pr_input, options.origin.as_deref()) { Ok(pr_info) => { spinner.success("Fetched PR metadata"); - return app::run_app_with_pr(options, pr_info, backend); + return app::run_app_with_pr(options, pr_info, backend, req_tx.clone(), res_rx); } Err(e) => { spinner.fail(&e); @@ -398,7 +519,7 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re match fetch_pr_info(input, options.origin.as_deref()) { Ok(pr_info) => { spinner.success("Fetched PR metadata"); - return app::run_app_with_pr(options, pr_info, backend); + return app::run_app_with_pr(options, pr_info, backend, req_tx.clone(), res_rx); } Err(e) => { spinner.fail(&e); @@ -443,12 +564,90 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re } }; - return app::run_app_stacked(options, commits, backend); + return app::run_app_stacked(options, commits, backend, req_tx.clone(), res_rx); } else { eprintln!("\x1b[91merror:\x1b[0m --stacked requires a range (e.g., main..feature)"); process::exit(1); } } - app::run_app(options, None, backend) + app::run_app(options, None, backend, req_tx, res_rx) +} + +#[cfg(test)] +mod tests { + use super::types::{FileDiff, FileStatus}; + use super::*; + + #[test] + fn combined_unified_diff_emits_a_diff_git_header_per_file() { + let file_diffs = vec![FileDiff { + filename: "src/a.rs".to_string(), + old_content: "old\n".to_string(), + new_content: "new\n".to_string(), + status: FileStatus::Modified, + is_binary: false, + }]; + + let combined = combined_unified_diff(&file_diffs); + + assert!(combined.contains("diff --git a/src/a.rs b/src/a.rs\n")); + assert!(combined.contains("-old\n")); + assert!(combined.contains("+new\n")); + } + + #[test] + fn combined_unified_diff_concatenates_headers_for_multiple_files_in_order() { + let file_diffs = vec![ + FileDiff { + filename: "src/a.rs".to_string(), + old_content: "old a\n".to_string(), + new_content: "new a\n".to_string(), + status: FileStatus::Modified, + is_binary: false, + }, + FileDiff { + filename: "src/b.rs".to_string(), + old_content: "old b\n".to_string(), + new_content: "new b\n".to_string(), + status: FileStatus::Modified, + is_binary: false, + }, + ]; + + let combined = combined_unified_diff(&file_diffs); + + let a_pos = combined + .find("diff --git a/src/a.rs b/src/a.rs\n") + .expect("missing header for a.rs"); + let b_pos = combined + .find("diff --git a/src/b.rs b/src/b.rs\n") + .expect("missing header for b.rs"); + assert!(a_pos < b_pos); + } + + #[test] + fn combined_unified_diff_skips_binary_files() { + let file_diffs = vec![ + FileDiff { + filename: "image.png".to_string(), + old_content: String::new(), + new_content: String::new(), + status: FileStatus::Added, + is_binary: true, + }, + FileDiff { + filename: "src/a.rs".to_string(), + old_content: "old\n".to_string(), + new_content: "new\n".to_string(), + status: FileStatus::Modified, + is_binary: false, + }, + ]; + + let combined = combined_unified_diff(&file_diffs); + + assert!(!combined.contains("image.png")); + assert!(combined.contains("diff --git a/src/a.rs b/src/a.rs\n")); + } } diff --git a/src/command/diff/render/diff_view.rs b/src/command/diff/render/diff_view.rs index 79f6e247..eb84acb2 100644 --- a/src/command/diff/render/diff_view.rs +++ b/src/command/diff/render/diff_view.rs @@ -8,9 +8,10 @@ use ratatui::{ use crate::command::diff::context::{compute_context_lines, ContextLine}; use crate::command::diff::highlight::{highlight_line_spans, FileHighlighter}; use crate::command::diff::search::{MatchPanel, SearchState}; -use crate::command::diff::state::{Annotation, AnnotationTarget}; +use crate::command::diff::state::{Annotation, AnnotationTarget, SidebarMode}; use crate::command::diff::theme; use crate::command::diff::{annotation::AnnotationEditor, state::TreeCache}; +use crate::grouped_summary::DiffGroup; /// One overlay slot in the rendered diff: either a saved annotation or the /// active inline editor (new or editing an existing annotation). @@ -89,7 +90,7 @@ use crate::command::diff::types::{ use crate::command::diff::PrInfo; use super::footer::{render_footer, FooterData}; -use super::sidebar::render_sidebar; +use super::sidebar::{render_guide_sidebar, render_sidebar, GuideStatus}; /// Render the header bar for stacked diff mode showing commit info with navigation arrows fn render_stacked_header( @@ -859,7 +860,6 @@ fn apply_selection_to_spans<'a>( result } - pub fn compute_line_stats(side_by_side: &[DiffLine]) -> LineStats { let mut added = 0; let mut removed = 0; @@ -994,7 +994,13 @@ fn compute_target_index_ranges( ) -> Vec<(usize, usize, DiffPanelFocus)> { let mut ranges = Vec::new(); for target in targets { - if let AnnotationTarget::LineRange { panel, start_line, end_line, .. } = target { + if let AnnotationTarget::LineRange { + panel, + start_line, + end_line, + .. + } = target + { let mut first_idx: Option = None; let mut last_idx: Option = None; for (idx, dl) in side_by_side.iter().enumerate() { @@ -1285,6 +1291,11 @@ pub fn render_diff( total_added: usize, total_removed: usize, editor: Option<&AnnotationEditor>, + sidebar_mode: SidebarMode, + guide_groups: &[DiffGroup], + guide_group_selected: usize, + guide_file_selected: usize, + guide_status: GuideStatus, ) -> (usize, Vec<(usize, usize)>, Vec<(u64, Rect)>, Option) { let area = frame.area(); let t = theme::get(); @@ -1328,22 +1339,36 @@ pub fn render_diff( .constraints([Constraint::Length(sidebar_width), Constraint::Min(0)]) .split(content_area); - render_sidebar( - frame, - main_chunks[0], - sidebar_items, - sidebar_visible, - collapsed_dirs, - current_file, - sidebar_selected, - sidebar_scroll, - sidebar_h_scroll, - viewed_files, - focused_panel == FocusedPanel::Sidebar, - _file_diffs.len(), - total_added, - total_removed, - ); + if sidebar_mode == SidebarMode::Guide { + render_guide_sidebar( + frame, + main_chunks[0], + guide_groups, + guide_group_selected, + guide_file_selected, + _file_diffs, + viewed_files, + focused_panel == FocusedPanel::Sidebar, + guide_status, + ); + } else { + render_sidebar( + frame, + main_chunks[0], + sidebar_items, + sidebar_visible, + collapsed_dirs, + current_file, + sidebar_selected, + sidebar_scroll, + sidebar_h_scroll, + viewed_files, + focused_panel == FocusedPanel::Sidebar, + _file_diffs.len(), + total_added, + total_removed, + ); + } main_chunks[1] } else { @@ -1550,7 +1575,10 @@ pub fn render_diff( // Check if this line is the end_line for any line-range slot for slot in &line_slots { - if let AnnotationTarget::LineRange { panel, end_line, .. } = slot.target() { + if let AnnotationTarget::LineRange { + panel, end_line, .. + } = slot.target() + { if diff_line.line_number(*panel) == Some(*end_line) { let num_ann_lines = slot.height(); let line_pos = new_lines.len(); @@ -1726,7 +1754,10 @@ pub fn render_diff( // Check if this line is the end_line for any line-range slot for slot in &line_slots { - if let AnnotationTarget::LineRange { panel, end_line, .. } = slot.target() { + if let AnnotationTarget::LineRange { + panel, end_line, .. + } = slot.target() + { if diff_line.line_number(*panel) == Some(*end_line) { let num_ann_lines = slot.height(); let line_pos = old_lines.len(); @@ -2195,7 +2226,10 @@ pub fn render_diff( // Check if this line is the end_line for any line-range slot for slot in &line_slots { - if let AnnotationTarget::LineRange { panel, end_line, .. } = slot.target() { + if let AnnotationTarget::LineRange { + panel, end_line, .. + } = slot.target() + { if diff_line.line_number(*panel) == Some(*end_line) { let num_lines = slot.height(); @@ -2395,6 +2429,10 @@ pub fn render_diff( }, ); - (content_row_offset, overlay_gaps, annotation_rects, editor_rect) + ( + content_row_offset, + overlay_gaps, + annotation_rects, + editor_rect, + ) } - diff --git a/src/command/diff/render/mod.rs b/src/command/diff/render/mod.rs index 1a79b638..97d395dd 100644 --- a/src/command/diff/render/mod.rs +++ b/src/command/diff/render/mod.rs @@ -9,5 +9,6 @@ pub use modal::{ FilePickerItem, FileStatus as ModalFileStatus, KeyBind, KeyBindSection, Modal, ModalContent, ModalResult, }; +pub use sidebar::GuideStatus; pub use crate::command::diff::global_search::GlobalSearchState; diff --git a/src/command/diff/render/modal.rs b/src/command/diff/render/modal.rs index b2c8fc2b..8a824ab8 100644 --- a/src/command/diff/render/modal.rs +++ b/src/command/diff/render/modal.rs @@ -1,7 +1,10 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseEvent, MouseEventKind}; use ratatui::{ prelude::*, - widgets::{Block, Borders, Clear, List, ListItem, Paragraph, Scrollbar, ScrollbarOrientation, ScrollbarState}, + widgets::{ + Block, Borders, Clear, List, ListItem, Paragraph, Scrollbar, ScrollbarOrientation, + ScrollbarState, + }, }; use crate::command::diff::global_search::{GlobalSearchState, LineChange}; @@ -39,8 +42,14 @@ pub enum FileStatus { pub enum ModalContent { #[allow(dead_code)] - Info { title: String, message: String }, - Confirm { title: String, message: String }, + Info { + title: String, + message: String, + }, + Confirm { + title: String, + message: String, + }, #[allow(dead_code)] Select { title: String, @@ -94,9 +103,15 @@ pub enum ModalResult { sbs_line_index: usize, panel: MatchPanel, }, - AnnotationJump { annotation_id: u64 }, - AnnotationEdit { annotation_id: u64 }, - AnnotationDelete { annotation_id: u64 }, + AnnotationJump { + annotation_id: u64, + }, + AnnotationEdit { + annotation_id: u64, + }, + AnnotationDelete { + annotation_id: u64, + }, AnnotationCopyAll, AnnotationExport(String), } @@ -235,7 +250,9 @@ impl Modal { (width, height) } ModalContent::Annotations { - items, export_input, .. + items, + export_input, + .. } => { let width = 100.min(area.width.saturating_sub(4)); let items_count = items.len().min(12) as u16; @@ -268,8 +285,20 @@ impl Modal { } => { self.render_select(frame, modal_area, title, items, *selected); } - ModalContent::KeyBindings { title, sections, scroll, content_height } => { - self.render_keybindings(frame, modal_area, title, sections, *scroll, *content_height); + ModalContent::KeyBindings { + title, + sections, + scroll, + content_height, + } => { + self.render_keybindings( + frame, + modal_area, + title, + sections, + *scroll, + *content_height, + ); } ModalContent::FilePicker { title, @@ -296,7 +325,15 @@ impl Modal { error_message, .. } => { - self.render_annotations(frame, modal_area, title, items, *selected, export_input.as_deref(), error_message.as_deref()); + self.render_annotations( + frame, + modal_area, + title, + items, + *selected, + export_input.as_deref(), + error_message.as_deref(), + ); } ModalContent::GlobalSearch { .. } => unreachable!(), } @@ -361,7 +398,10 @@ impl Modal { // Thin separator under the prompt let sep = "─".repeat(left_rows[1].width as usize); frame.render_widget( - Paragraph::new(Span::styled(sep, Style::default().fg(t.ui.border_unfocused))), + Paragraph::new(Span::styled( + sep, + Style::default().fg(t.ui.border_unfocused), + )), left_rows[1], ); @@ -559,7 +599,12 @@ impl Modal { } // Reserve space for scrollbar on the right - let content_area = Rect::new(inner.x, inner.y, inner.width.saturating_sub(1), inner.height); + let content_area = Rect::new( + inner.x, + inner.y, + inner.width.saturating_sub(1), + inner.height, + ); let para = Paragraph::new(lines).scroll((scroll, 0)); frame.render_widget(para, content_area); @@ -573,8 +618,9 @@ impl Modal { .track_symbol(Some("│")) .thumb_symbol("█"); - let mut scrollbar_state = ScrollbarState::new(content_height.saturating_sub(visible_height) as usize) - .position(scroll as usize); + let mut scrollbar_state = + ScrollbarState::new(content_height.saturating_sub(visible_height) as usize) + .position(scroll as usize); frame.render_stateful_widget(scrollbar, inner, &mut scrollbar_state); } @@ -749,20 +795,24 @@ impl Modal { let content_width = available_width.saturating_sub(time_width + 4); // 4 for padding/separators // Allocate: 45% for location, 55% for preview (minimum 20 chars each if space allows) - let location_max = (content_width * 45 / 100).max(20).min(content_width.saturating_sub(20)); + let location_max = (content_width * 45 / 100) + .max(20) + .min(content_width.saturating_sub(20)); let preview_max = content_width.saturating_sub(location_max); // Truncate location if needed (using char count for proper UTF-8 handling) - let truncated_location = if location.chars().count() > location_max && location_max > 3 { - let truncate_at = location_max - 1; - let truncated: String = location.chars().take(truncate_at).collect(); - format!("{}…", truncated) - } else { - location.to_string() - }; + let truncated_location = + if location.chars().count() > location_max && location_max > 3 { + let truncate_at = location_max - 1; + let truncated: String = location.chars().take(truncate_at).collect(); + format!("{}…", truncated) + } else { + location.to_string() + }; // Truncate preview if needed (using char count for proper UTF-8 handling) - let truncated_preview = if preview.chars().count() > preview_max && preview_max > 3 { + let truncated_preview = if preview.chars().count() > preview_max && preview_max > 3 + { let truncate_at = preview_max - 1; let truncated: String = preview.chars().take(truncate_at).collect(); format!("{}…", truncated) @@ -772,7 +822,7 @@ impl Modal { // Calculate padding to right-align time (using char count for proper width calculation) let location_len = truncated_location.chars().count() + 2; // " location " - let preview_len = truncated_preview.chars().count() + 1; // " preview" + let preview_len = truncated_preview.chars().count() + 1; // " preview" let used_width = location_len + preview_len + time_width; let padding = available_width.saturating_sub(used_width); @@ -808,14 +858,8 @@ impl Modal { format!(" {}", truncated_preview), Style::default().fg(t.ui.text_muted).italic(), ), - Span::styled( - format!("{:>width$}", "", width = padding), - Style::default(), - ), - Span::styled( - format!(" {} ", time), - Style::default().fg(t.ui.text_muted), - ), + Span::styled(format!("{:>width$}", "", width = padding), Style::default()), + Span::styled(format!(" {} ", time), Style::default().fg(t.ui.text_muted)), ] }; @@ -861,9 +905,15 @@ impl Modal { Span::styled("Error: ", Style::default().fg(t.ui.status_deleted).bold()), Span::styled(error, Style::default().fg(t.ui.status_deleted)), ]); - let error_para = Paragraph::new(error_line).alignment(ratatui::prelude::Alignment::Center); + let error_para = + Paragraph::new(error_line).alignment(ratatui::prelude::Alignment::Center); // Render error in the list area's last line - let error_area = Rect::new(list_area.x, list_area.y + list_area.height.saturating_sub(1), list_area.width, 1); + let error_area = Rect::new( + list_area.x, + list_area.y + list_area.height.saturating_sub(1), + list_area.width, + 1, + ); frame.render_widget(error_para, error_area); } @@ -952,10 +1002,7 @@ impl Modal { // user can verify before pressing Enter to jump. // Clicks outside the list rows (border / prompt / // separator / right pane) do nothing. - if in_left - && mouse.row >= list_y_start - && mouse.row + 1 < terminal_height - { + if in_left && mouse.row >= list_y_start && mouse.row + 1 < terminal_height { let row_in_list = (mouse.row - list_y_start) as usize; let target = state.list_scroll + row_in_list; if target < state.results.len() { @@ -1071,8 +1118,13 @@ impl Modal { } _ => None, }, - ModalContent::KeyBindings { scroll, content_height, .. } => { - let visible_height = calculate_keybindings_visible_height(terminal_height, *content_height); + ModalContent::KeyBindings { + scroll, + content_height, + .. + } => { + let visible_height = + calculate_keybindings_visible_height(terminal_height, *content_height); let max_scroll = content_height.saturating_sub(visible_height); match key.code { @@ -1245,9 +1297,7 @@ impl Modal { } else { // Normal mode match key.code { - KeyCode::Esc - | KeyCode::Char('q') - | KeyCode::Char('c') + KeyCode::Esc | KeyCode::Char('q') | KeyCode::Char('c') if key.code == KeyCode::Esc || key.code == KeyCode::Char('q') || key.modifiers.contains(KeyModifiers::CONTROL) => @@ -1264,21 +1314,27 @@ impl Modal { *selected = selected.saturating_sub(1); None } - KeyCode::Enter => annotations.get(*selected).map(|ann| { - ModalResult::AnnotationJump { - annotation_id: ann.id, - } - }), - KeyCode::Char('e') => annotations.get(*selected).map(|ann| { - ModalResult::AnnotationEdit { - annotation_id: ann.id, - } - }), - KeyCode::Char('d') => annotations.get(*selected).map(|ann| { - ModalResult::AnnotationDelete { - annotation_id: ann.id, - } - }), + KeyCode::Enter => { + annotations + .get(*selected) + .map(|ann| ModalResult::AnnotationJump { + annotation_id: ann.id, + }) + } + KeyCode::Char('e') => { + annotations + .get(*selected) + .map(|ann| ModalResult::AnnotationEdit { + annotation_id: ann.id, + }) + } + KeyCode::Char('d') => { + annotations + .get(*selected) + .map(|ann| ModalResult::AnnotationDelete { + annotation_id: ann.id, + }) + } KeyCode::Char('y') => Some(ModalResult::AnnotationCopyAll), KeyCode::Char('o') => { *export_input = Some(String::from("annotations.txt")); @@ -1307,8 +1363,8 @@ impl Modal { let is_up = matches!(key.code, KeyCode::Up) || (ctrl && matches!(key.code, KeyCode::Char('p') | KeyCode::Char('k'))); // Half-page down — PageDown / Ctrl+d. (Ctrl+u is taken by clear-query.) - let is_page_down = - matches!(key.code, KeyCode::PageDown) || (ctrl && matches!(key.code, KeyCode::Char('d'))); + let is_page_down = matches!(key.code, KeyCode::PageDown) + || (ctrl && matches!(key.code, KeyCode::Char('d'))); if is_down { state.move_down(visible_rows); @@ -1478,7 +1534,10 @@ fn build_result_row( // Selector + change symbol — fixed prefix that doesn't horizontally scroll. let mut spans: Vec> = Vec::with_capacity(16); let selector = if is_selected { - Span::styled("❯ ", apply_bg(Style::default().fg(t.ui.border_focused).bold())) + Span::styled( + "❯ ", + apply_bg(Style::default().fg(t.ui.border_focused).bold()), + ) } else { Span::styled(" ", apply_bg(Style::default())) }; @@ -1588,10 +1647,7 @@ fn render_preview_pane( if !state.query.is_empty() { let msg = Line::from(Span::styled( " no matches", - Style::default() - .fg(t.ui.text_muted) - .bg(t.ui.bg) - .italic(), + Style::default().fg(t.ui.text_muted).bg(t.ui.bg).italic(), )); frame.render_widget(Paragraph::new(msg), area); } @@ -1662,7 +1718,11 @@ fn render_preview_pane( if is_cursor { cursor_idx = Some(metas.len()); } - metas.push(PreviewRowMeta { sbs_idx, side: Side::New, is_cursor }); + metas.push(PreviewRowMeta { + sbs_idx, + side: Side::New, + is_cursor, + }); } } ChangeType::Insert => { @@ -1680,7 +1740,11 @@ fn render_preview_pane( if is_cursor { cursor_idx = Some(metas.len()); } - metas.push(PreviewRowMeta { sbs_idx, side: Side::Old, is_cursor }); + metas.push(PreviewRowMeta { + sbs_idx, + side: Side::Old, + is_cursor, + }); } } ChangeType::Modified => { @@ -1689,7 +1753,11 @@ fn render_preview_pane( if is_cursor { cursor_idx = Some(metas.len()); } - metas.push(PreviewRowMeta { sbs_idx, side: Side::Old, is_cursor }); + metas.push(PreviewRowMeta { + sbs_idx, + side: Side::Old, + is_cursor, + }); } if sbs_line.new_line.is_some() { pending.push(PreviewRowMeta { @@ -1819,10 +1887,7 @@ fn make_preview_row<'a>( // Left-edge cursor accent: a single thin vertical block on the matched row, // a blank cell otherwise. Sits flush against the pane's left edge. let accent = if is_cursor { - Span::styled( - "▌", - Style::default().fg(t.ui.border_focused).bg(gutter_bg), - ) + Span::styled("▌", Style::default().fg(t.ui.border_focused).bg(gutter_bg)) } else { Span::styled(" ", Style::default().bg(gutter_bg)) }; diff --git a/src/command/diff/render/sidebar.rs b/src/command/diff/render/sidebar.rs index 84dbd6e8..a20416f9 100644 --- a/src/command/diff/render/sidebar.rs +++ b/src/command/diff/render/sidebar.rs @@ -5,8 +5,93 @@ use ratatui::{ widgets::{Block, Borders, Paragraph}, }; -use crate::command::diff::theme; -use crate::command::diff::types::{FileStatus, SidebarItem}; +use crate::command::diff::theme::{self, Theme}; +use crate::command::diff::types::{FileDiff, FileStatus, SidebarItem}; +use crate::grouped_summary::DiffGroup; + +/// Greedy word-wrap at `width` columns. Words longer than `width` are left +/// unbroken (overflow rather than mid-word split). +pub(super) fn wrap_plain(text: &str, width: usize) -> Vec { + if text.is_empty() { + return vec![String::new()]; + } + let mut lines = Vec::new(); + let mut current = String::new(); + for word in text.split_whitespace() { + if current.is_empty() { + current.push_str(word); + } else if current.len() + 1 + word.len() <= width { + current.push(' '); + current.push_str(word); + } else { + lines.push(std::mem::take(&mut current)); + current.push_str(word); + } + } + if !current.is_empty() { + lines.push(current); + } + if lines.is_empty() { + lines.push(String::new()); + } + lines +} + +/// Render a single file row (icon/viewed-marker/status-color/name), shared +/// by the directory-tree sidebar and the Guide sidebar's per-group file +/// list so both stay visually consistent. +#[allow(clippy::too_many_arguments)] +fn file_row_line( + theme: &Theme, + indent_depth: usize, + name: &str, + status: FileStatus, + viewed: bool, + is_current: bool, + is_selected: bool, + is_focused: bool, +) -> Line<'static> { + let indent = " ".repeat(indent_depth); + let marker = if viewed { "✓ " } else { " " }; + let status_color = match status { + FileStatus::Modified => Some(theme.ui.status_modified), + FileStatus::Added => Some(theme.ui.status_added), + FileStatus::Deleted => Some(theme.ui.status_deleted), + }; + let status_symbol = status.symbol().to_string(); + let prefix = format!("{}{}", indent, marker); + let name = format!(" {}", name); + + let base_style = if is_selected { + Style::default() + .fg(theme.ui.selection_fg) + .bg(if is_focused { + theme.ui.selection_bg + } else { + theme.ui.border_unfocused + }) + } else if is_current { + Style::default().fg(theme.ui.highlight) + } else if viewed { + Style::default().fg(theme.ui.viewed) + } else { + Style::default() + }; + + let status_style = if is_selected { + base_style + } else if let Some(color) = status_color { + Style::default().fg(color) + } else { + base_style + }; + + Line::from(vec![ + Span::styled(prefix, base_style), + Span::styled(status_symbol, status_style), + Span::styled(name, base_style), + ]) +} #[allow(clippy::too_many_arguments)] pub fn render_sidebar( @@ -33,8 +118,8 @@ pub fn render_sidebar( .enumerate() .map(|(i, item_idx)| { let item = &sidebar_items[*item_idx]; - let (prefix, status_symbol, status_color, name, is_current_file, is_viewed) = match item - { + let is_selected = i == sidebar_selected; + match item { SidebarItem::Directory { name, path, depth, .. } => { @@ -76,14 +161,27 @@ pub fn render_sidebar( } else { " " }; - ( - format!("{}{}", indent, marker), - status_symbol.to_string(), - None, - format!(" {}", name), - false, - all_children_viewed && has_children, - ) + let is_viewed = all_children_viewed && has_children; + + let prefix = format!("{}{}", indent, marker); + let name = format!(" {}", name); + let base_style = if is_selected { + Style::default().fg(t.ui.selection_fg).bg(if is_focused { + t.ui.selection_bg + } else { + t.ui.border_unfocused + }) + } else if is_viewed { + Style::default().fg(t.ui.viewed) + } else { + Style::default() + }; + + Line::from(vec![ + Span::styled(prefix, base_style), + Span::styled(status_symbol.to_string(), base_style), + Span::styled(name, base_style), + ]) } SidebarItem::File { name, @@ -91,55 +189,17 @@ pub fn render_sidebar( depth, status, .. - } => { - let indent = " ".repeat(*depth); - let viewed = viewed_files.contains(file_index); - let marker = if viewed { "✓ " } else { " " }; - let status_color = match status { - FileStatus::Modified => Some(t.ui.status_modified), - FileStatus::Added => Some(t.ui.status_added), - FileStatus::Deleted => Some(t.ui.status_deleted), - }; - let status_symbol = status.symbol().to_string(); - ( - format!("{}{}", indent, marker), - status_symbol, - status_color, - format!(" {}", name), - *file_index == current_file, - viewed, - ) - } - }; - - let is_selected = i == sidebar_selected; - let base_style = if is_selected { - Style::default().fg(t.ui.selection_fg).bg(if is_focused { - t.ui.selection_bg - } else { - t.ui.border_unfocused - }) - } else if is_current_file { - Style::default().fg(t.ui.highlight) - } else if is_viewed { - Style::default().fg(t.ui.viewed) - } else { - Style::default() - }; - - let status_style = if is_selected { - base_style - } else if let Some(color) = status_color { - Style::default().fg(color) - } else { - base_style - }; - - Line::from(vec![ - Span::styled(prefix, base_style), - Span::styled(status_symbol, status_style), - Span::styled(name, base_style), - ]) + } => file_row_line( + t, + *depth, + name, + *status, + viewed_files.contains(file_index), + *file_index == current_file, + is_selected, + is_focused, + ), + } }) .collect(); @@ -190,3 +250,125 @@ pub fn render_sidebar( frame.render_widget(para, area); } + +/// Availability of the current diff's grouped-summary data — drives what +/// `render_guide_sidebar` shows in place of the group/file list. +#[derive(Clone)] +pub enum GuideStatus { + Ready, + Pending, + Error(String), + Empty, + Disabled, +} + +/// Render the Guide sidebar: the AI-generated grouping of the current diff, +/// one group at a time, with its file list underneath. Replaces +/// `render_sidebar` in the same panel slot when `SidebarMode::Guide` is +/// active. No scrollbar in v1 — content clips on overflow. +#[allow(clippy::too_many_arguments)] +pub fn render_guide_sidebar( + frame: &mut Frame, + area: Rect, + groups: &[DiffGroup], + group_selected: usize, + guide_file_selected: usize, + file_diffs: &[FileDiff], + viewed_files: &HashSet, + is_focused: bool, + status: GuideStatus, +) { + let t = theme::get(); + let bg = t.ui.bg; + let title_style = if is_focused { + Style::default().fg(t.ui.border_focused) + } else { + Style::default().fg(t.ui.border_unfocused) + }; + let border_style = Style::default().fg(t.ui.border_unfocused); + let muted_style = Style::default().fg(t.ui.text_muted); + + let title = Line::from(vec![Span::styled(" [1] Guide ", title_style)]); + // Same border convention as `render_sidebar`: no right border, since the + // adjacent diff panel's left border stands in for it. + let borders = Borders::TOP | Borders::LEFT | Borders::BOTTOM; + let block = Block::default() + .title(title) + .borders(borders) + .border_style(border_style) + .style(Style::default().bg(bg)); + let inner_width = block.inner(area).width.saturating_sub(1).max(1) as usize; + + let lines: Vec = match status { + GuideStatus::Pending => vec![Line::from(Span::styled("Generating guide…", muted_style))], + GuideStatus::Error(e) => { + vec![Line::from(Span::styled( + format!("Guide failed: {e}"), + muted_style, + ))] + } + GuideStatus::Empty => vec![Line::from(Span::styled("No groups", muted_style))], + GuideStatus::Disabled => vec![Line::from(Span::styled("Guide disabled", muted_style))], + GuideStatus::Ready if groups.is_empty() => { + vec![Line::from(Span::styled("No groups", muted_style))] + } + GuideStatus::Ready => { + let group_selected = group_selected.min(groups.len() - 1); + let group = &groups[group_selected]; + let mut lines = Vec::new(); + + lines.push(Line::from(Span::styled( + format!("{:02} / {:02}", group_selected + 1, groups.len()), + muted_style, + ))); + lines.push(Line::from("")); + for wrapped in wrap_plain(&group.title, inner_width) { + lines.push(Line::from(Span::styled( + wrapped, + Style::default().fg(t.ui.text_primary).bold(), + ))); + } + lines.push(Line::from("")); + for wrapped in wrap_plain(&group.summary, inner_width) { + lines.push(Line::from(Span::styled( + wrapped, + Style::default().fg(t.ui.text_primary), + ))); + } + lines.push(Line::from("")); + for (idx, filename) in group.files.iter().enumerate() { + let is_selected = idx == guide_file_selected; + match file_diffs.iter().position(|f| &f.filename == filename) { + Some(file_index) => { + lines.push(file_row_line( + t, + 0, + filename, + file_diffs[file_index].status, + viewed_files.contains(&file_index), + false, + is_selected, + is_focused, + )); + } + // File named in the group no longer matches any current + // file_diffs entry (e.g. reconciliation left a stale + // name) — show it plainly rather than dropping it. + None => { + lines.push(Line::from(Span::styled( + format!(" {}", filename), + muted_style, + ))); + } + } + } + lines + } + }; + + let para = Paragraph::new(lines) + .style(Style::default().bg(bg)) + .block(block); + + frame.render_widget(para, area); +} diff --git a/src/command/diff/state.rs b/src/command/diff/state.rs index 5d2f7b6a..8c6f80d5 100644 --- a/src/command/diff/state.rs +++ b/src/command/diff/state.rs @@ -4,13 +4,15 @@ use std::time::SystemTime; use tree_sitter::{Parser, Tree}; use crate::command::diff::context::get_language_context; -use crate::command::diff::diff_algo::{compute_side_by_side, count_added_removed, find_hunk_starts}; +use crate::command::diff::diff_algo::{ + compute_side_by_side, count_added_removed, find_hunk_starts, +}; use crate::command::diff::highlight::FileHighlighter; use crate::command::diff::search::SearchState; use crate::command::diff::types::{ - build_file_tree, CursorPosition, DiffFullscreen, DiffLine, DiffPanelFocus, - DiffViewSettings, FileDiff, FocusedPanel, Selection, SelectionMode, SidebarItem, + build_file_tree, CursorPosition, DiffFullscreen, DiffLine, DiffPanelFocus, DiffViewSettings, + FileDiff, FocusedPanel, Selection, SelectionMode, SidebarItem, }; use crate::vcs::StackedCommitInfo; @@ -21,6 +23,15 @@ pub enum PendingKey { G, } +/// Which content the sidebar panel is currently showing: the normal +/// directory/file tree, or the AI-generated review guide (grouped changes). +#[derive(Default, Clone, Copy, PartialEq)] +pub enum SidebarMode { + #[default] + Directory, + Guide, +} + fn sidebar_item_path(item: &SidebarItem) -> &str { match item { SidebarItem::Directory { path, .. } => path, @@ -108,7 +119,10 @@ impl Annotation { #[cfg(not(feature = "jj"))] pub fn format_time(&self) -> String { use std::time::UNIX_EPOCH; - let duration = self.created_at.duration_since(UNIX_EPOCH).unwrap_or_default(); + let duration = self + .created_at + .duration_since(UNIX_EPOCH) + .unwrap_or_default(); let secs = duration.as_secs(); let hours = (secs / 3600) % 24; let minutes = (secs / 60) % 60; @@ -131,7 +145,11 @@ impl Annotation { pub fn line_range_display(&self) -> String { match &self.target { AnnotationTarget::File => String::new(), - AnnotationTarget::LineRange { start_line, end_line, .. } => { + AnnotationTarget::LineRange { + start_line, + end_line, + .. + } => { if start_line == end_line { format!("L{}", start_line) } else { @@ -227,6 +245,24 @@ pub struct AppState { pub total_added: usize, /// Total removed lines across all files in the current diff. Recomputed on reload. pub total_removed: usize, + /// AI-generated groupings, keyed by diff identity (see + /// `current_diff_identity` in `app.rs`). Self-invalidating: a changed + /// diff produces a new identity, so stale entries for old identities + /// are simply never looked up again — no explicit pruning needed. + pub groups_cache: HashMap, + /// Identities with a grouped-summary request currently in flight. + pub groups_pending: HashSet, + /// Error messages from failed grouped-summary requests, keyed by identity. + pub groups_errors: HashMap, + /// Identity of the diff currently displayed; set by + /// `maybe_trigger_group_generation`. + pub current_diff_identity: String, + /// Whether the sidebar is showing the directory tree or the Guide. + pub sidebar_mode: SidebarMode, + /// Selected group index within the current diff's Guide. + pub guide_group_selected: usize, + /// Selected file index within the currently selected Guide group. + pub guide_file_selected: usize, } fn compute_total_line_stats(file_diffs: &[FileDiff]) -> (usize, usize) { @@ -330,6 +366,13 @@ impl AppState { editor_rect: None, total_added, total_removed, + groups_cache: HashMap::new(), + groups_pending: HashSet::new(), + groups_errors: HashMap::new(), + current_diff_identity: String::new(), + sidebar_mode: SidebarMode::default(), + guide_group_selected: 0, + guide_file_selected: 0, } } @@ -575,7 +618,12 @@ impl AppState { } /// Start a new selection - pub fn start_selection(&mut self, panel: DiffPanelFocus, pos: CursorPosition, mode: SelectionMode) { + pub fn start_selection( + &mut self, + panel: DiffPanelFocus, + pos: CursorPosition, + mode: SelectionMode, + ) { self.diff_panel_focus = panel; self.selection = Selection { panel, @@ -808,9 +856,15 @@ impl AppState { self.sidebar_items = build_file_tree(&self.file_diffs); // Retain annotations whose file still exists - let filenames: HashSet<&str> = self.file_diffs.iter().map(|f| f.filename.as_str()).collect(); - self.annotations.retain(|ann| filenames.contains(ann.filename.as_str())); - self.viewed_hunks.retain(|fname, _| filenames.contains(fname.as_str())); + let filenames: HashSet<&str> = self + .file_diffs + .iter() + .map(|f| f.filename.as_str()) + .collect(); + self.annotations + .retain(|ann| filenames.contains(ann.filename.as_str())); + self.viewed_hunks + .retain(|fname, _| filenames.contains(fname.as_str())); // Convert viewed filenames back to indices in the new file_diffs self.viewed_files = self @@ -838,6 +892,15 @@ impl AppState { self.needs_reload = false; self.invalidate_cache(); // Clear cache after reload + // The diff content may have changed underneath the current Guide + // selection; reset it rather than leaving indices dangling into a + // group/file list that may no longer match. The grouped-summary + // cache itself needs no clearing here — it's keyed by + // content-derived identity, so a changed diff simply gets a new + // key and the trigger helper re-requests it. + self.guide_group_selected = 0; + self.guide_file_selected = 0; + // Preserve scroll position instead of resetting if !self.file_diffs.is_empty() { // Keep the old scroll position, but clamp to valid range @@ -848,6 +911,11 @@ impl AppState { } } + /// Find the index into `file_diffs` for a given filename, if present. + pub fn file_index_for_path(&self, path: &str) -> Option { + self.file_diffs.iter().position(|f| f.filename == path) + } + pub fn select_file(&mut self, file_index: usize) { self.current_file = file_index; self.diff_fullscreen = DiffFullscreen::None; @@ -879,7 +947,13 @@ impl AppState { } /// Add a new annotation, returns its id - pub fn add_annotation(&mut self, filename: String, target: AnnotationTarget, content: String, created_at: SystemTime) -> u64 { + pub fn add_annotation( + &mut self, + filename: String, + target: AnnotationTarget, + content: String, + created_at: SystemTime, + ) -> u64 { let id = self.annotation_next_id; self.annotation_next_id += 1; self.annotations.push(Annotation { @@ -925,7 +999,12 @@ impl AppState { AnnotationTarget::File => { result.push_str(&format!("**{}**\n\n", ann.filename)); } - AnnotationTarget::LineRange { panel, start_line, end_line, .. } => { + AnnotationTarget::LineRange { + panel, + start_line, + end_line, + .. + } => { let side = match panel { DiffPanelFocus::Old => "LEFT", _ => "RIGHT", @@ -952,6 +1031,30 @@ impl AppState { result.trim_end().to_string() } } + +/// Mark each of `paths` as viewed in `state`, set-only (never unmarks an +/// already-viewed file). Unknown paths are silently ignored. In PR mode +/// (`pr_info.is_some()`), newly-marked files also fire the async +/// mark-as-viewed GitHub mutation. +/// +/// This is a standalone helper for the AI-grouping "mark group viewed" +/// action; it intentionally does not share code with the Space-key +/// bulk-toggle handler in `app.rs`, which has its own toggle (not set-only) +/// semantics. +pub fn mark_paths_viewed(state: &mut AppState, paths: &[String], pr_info: Option<&super::PrInfo>) { + for path in paths { + let Some(idx) = state.file_index_for_path(path) else { + continue; + }; + let newly_viewed = state.viewed_files.insert(idx); + if newly_viewed { + if let Some(pr) = pr_info { + super::mark_file_as_viewed_async(pr, path); + } + } + } +} + pub fn adjust_scroll_to_line( line: usize, scroll: u16, @@ -1062,7 +1165,9 @@ mod tests { let state = AppState::new(diffs, Some("ccc.rs")); - if let Some(SidebarItem::File { file_index, .. }) = state.sidebar_item_at_visible(state.sidebar_selected) { + if let Some(SidebarItem::File { file_index, .. }) = + state.sidebar_item_at_visible(state.sidebar_selected) + { assert_eq!(*file_index, state.current_file); } else { panic!("sidebar_selected should point to a file"); @@ -1078,4 +1183,61 @@ mod tests { assert_eq!(state.current_file, 0); assert!(state.file_diffs.is_empty()); } + + #[test] + fn file_index_for_path_returns_index_of_matching_file() { + let diffs = vec![make_file_diff("src/a.rs"), make_file_diff("src/b.rs")]; + let state = AppState::new(diffs, None); + + assert_eq!(state.file_index_for_path("src/b.rs"), Some(1)); + } + + #[test] + fn file_index_for_path_returns_none_for_unknown_path() { + let diffs = vec![make_file_diff("src/a.rs")]; + let state = AppState::new(diffs, None); + + assert_eq!(state.file_index_for_path("src/nonexistent.rs"), None); + } + + #[test] + fn mark_paths_viewed_marks_known_paths_by_index() { + let diffs = vec![ + make_file_diff("src/a.rs"), + make_file_diff("src/b.rs"), + make_file_diff("src/c.rs"), + ]; + let mut state = AppState::new(diffs, None); + + mark_paths_viewed( + &mut state, + &["src/a.rs".to_string(), "src/c.rs".to_string()], + None, + ); + + assert!(state.viewed_files.contains(&0)); + assert!(!state.viewed_files.contains(&1)); + assert!(state.viewed_files.contains(&2)); + } + + #[test] + fn mark_paths_viewed_ignores_unknown_paths_without_panicking() { + let diffs = vec![make_file_diff("src/a.rs")]; + let mut state = AppState::new(diffs, None); + + mark_paths_viewed(&mut state, &["does/not/exist.rs".to_string()], None); + + assert!(state.viewed_files.is_empty()); + } + + #[test] + fn mark_paths_viewed_leaves_already_viewed_files_viewed() { + let diffs = vec![make_file_diff("src/a.rs")]; + let mut state = AppState::new(diffs, None); + state.viewed_files.insert(0); + + mark_paths_viewed(&mut state, &["src/a.rs".to_string()], None); + + assert!(state.viewed_files.contains(&0)); + } } diff --git a/src/command/mod.rs b/src/command/mod.rs index 706ba35a..8d31435a 100644 --- a/src/command/mod.rs +++ b/src/command/mod.rs @@ -3,6 +3,7 @@ use explain::ExplainCommand; use list::ListCommand; use operate::OperateCommand; use std::process::Stdio; +use std::sync::Arc; use crate::config::configuration::DraftConfig; use crate::error::LumenError; @@ -37,11 +38,11 @@ pub enum CommandType<'a> { } pub struct LumenCommand { - provider: LumenProvider, + provider: Arc, } impl LumenCommand { - pub fn new(provider: LumenProvider) -> Self { + pub fn new(provider: Arc) -> Self { LumenCommand { provider } } diff --git a/src/config/cli.rs b/src/config/cli.rs index 5cb6fae5..cb42feac 100644 --- a/src/config/cli.rs +++ b/src/config/cli.rs @@ -151,6 +151,10 @@ pub enum Commands { /// Soft-wrap long diff lines instead of scrolling horizontally #[arg(long)] wrap: bool, + + /// Show an AI-generated review guide in the sidebar (grouped changes) + #[arg(long)] + guide: bool, }, /// Interactively configure Lumen (provider, API key) Configure, @@ -187,6 +191,15 @@ mod tests { } } + #[test] + fn test_diff_guide_flag_parses() { + let cli = Cli::try_parse_from(["lumen", "diff", "--guide"]).unwrap(); + match cli.command { + Commands::Diff { guide, .. } => assert!(guide), + _ => panic!("expected diff command"), + } + } + #[test] fn test_explain_grouped_flag_parses() { let cli = Cli::try_parse_from(["lumen", "explain", "--grouped"]).unwrap(); diff --git a/src/config/configuration.rs b/src/config/configuration.rs index 037a66d5..f25edf7d 100644 --- a/src/config/configuration.rs +++ b/src/config/configuration.rs @@ -32,6 +32,9 @@ pub struct LumenConfig { #[serde(default)] pub wrap: Option, + + #[serde(default)] + pub guide: Option, } #[derive(Debug, Deserialize, Default)] @@ -130,6 +133,7 @@ impl LumenConfig { draft: config.draft, theme: config.theme, wrap: config.wrap, + guide: config.guide, }) } @@ -156,6 +160,7 @@ impl Default for LumenConfig { draft: default_draft_config(), theme: None, wrap: None, + guide: None, } } } diff --git a/src/main.rs b/src/main.rs index 5aeeaab9..be803492 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ use error::LumenError; use git_entity::{commit::Commit, diff::Diff, GitEntity}; use std::io::Read; use std::process; +use std::sync::Arc; use vcs::VcsBackendType; mod ai_prompt; @@ -35,8 +36,12 @@ async fn run() -> Result<(), LumenError> { Err(e) => return Err(e), }; - let provider = provider::LumenProvider::new(config.provider, config.api_key, config.model)?; - let command = command::LumenCommand::new(provider); + let provider = Arc::new(provider::LumenProvider::new( + config.provider, + config.api_key, + config.model, + )?); + let command = command::LumenCommand::new(provider.clone()); // Get VCS backend based on CLI override or auto-detection let cwd = std::env::current_dir()?; @@ -139,6 +144,7 @@ async fn run() -> Result<(), LumenError> { focus, origin, wrap, + guide, } => { let options = command::diff::DiffOptions { reference, @@ -151,8 +157,9 @@ async fn run() -> Result<(), LumenError> { focus, origin, wrap: wrap || config.wrap.unwrap_or(false), + guide: guide || config.guide.unwrap_or(false), }; - command::diff::run_diff_ui(options, backend.as_ref())?; + command::diff::run_diff_ui(options, backend.as_ref(), provider.clone())?; } Commands::Configure => { command::configure::ConfigureCommand::execute()?; From 45d6bb7eb94952b3917b1108a9d0a7e5da2c9866 Mon Sep 17 00:00:00 2001 From: Andrew Thal <467872+athal7@users.noreply.github.com> Date: Tue, 14 Jul 2026 09:49:37 -0500 Subject: [PATCH 3/3] fix(diff): surface feedback when Guide mode has nothing to show Pressing `a` without --guide was a silent no-op, with no indication the flag was required. Also add a footer indicator while the AI grouping is generating, visible without toggling into the Guide sidebar first. Co-Authored-By: anthropic/claude-sonnet-5 --- src/command/diff/app.rs | 8 ++++- src/command/diff/render/diff_view.rs | 10 ++++++ src/command/diff/render/footer.rs | 48 ++++++++++++++++++++-------- src/command/diff/state.rs | 28 +++++++++++++++- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/src/command/diff/app.rs b/src/command/diff/app.rs index 0f5ef11c..4edcf7d9 100644 --- a/src/command/diff/app.rs +++ b/src/command/diff/app.rs @@ -489,6 +489,8 @@ fn run_app_internal( maybe_trigger_group_generation(&mut state, &options, &req_tx); 'main: loop { + state.expire_status_message(); + if let Some(ref rx) = watch_rx { match rx.try_recv() { Ok(event) => { @@ -639,6 +641,7 @@ fn run_app_internal( state.guide_group_selected, state.guide_file_selected, guide_status.clone(), + state.status_message.as_ref().map(|(msg, _)| msg.as_str()), ); row_offset.set(offset); *rects_cell.borrow_mut() = rects; @@ -1950,7 +1953,6 @@ fn run_app_internal( } } KeyCode::Char('a') => { - // Nothing to show without --guide: leave the sidebar alone. if options.guide { state.sidebar_mode = match state.sidebar_mode { SidebarMode::Directory => SidebarMode::Guide, @@ -1962,6 +1964,10 @@ fn run_app_internal( state.focused_panel = FocusedPanel::Sidebar; select_guide_file(&mut state, 0); } + } else { + // Nothing to show without --guide: say so instead + // of silently doing nothing. + state.set_status_message("Guide requires --guide flag"); } } KeyCode::Char(',') => { diff --git a/src/command/diff/render/diff_view.rs b/src/command/diff/render/diff_view.rs index eb84acb2..a8fb3f55 100644 --- a/src/command/diff/render/diff_view.rs +++ b/src/command/diff/render/diff_view.rs @@ -1296,11 +1296,17 @@ pub fn render_diff( guide_group_selected: usize, guide_file_selected: usize, guide_status: GuideStatus, + status_message: Option<&str>, ) -> (usize, Vec<(usize, usize)>, Vec<(u64, Rect)>, Option) { let area = frame.area(); let t = theme::get(); let bg = t.ui.bg; let h_scroll = if settings.wrap { 0 } else { h_scroll }; + // Compute before `guide_status` is potentially moved into + // `render_guide_sidebar` below — this drives the footer's ambient + // "generating" indicator, which is visible even outside the Guide + // sidebar. + let guide_pending = matches!(&guide_status, GuideStatus::Pending); // Layout: header (if stacked) + main content + footer let (content_area, footer_area) = if stacked_mode { @@ -1414,6 +1420,8 @@ pub fn render_diff( focused_hunk: None, search_state, area_width: area.width, + status_message, + guide_pending, }, ); return (0, Vec::new(), Vec::new(), None); @@ -2426,6 +2434,8 @@ pub fn render_diff( focused_hunk, search_state, area_width: area.width, + status_message, + guide_pending, }, ); diff --git a/src/command/diff/render/footer.rs b/src/command/diff/render/footer.rs index 47610cbb..a87bb19b 100644 --- a/src/command/diff/render/footer.rs +++ b/src/command/diff/render/footer.rs @@ -19,6 +19,14 @@ pub struct FooterData<'a> { pub focused_hunk: Option, pub search_state: &'a SearchState, pub area_width: u16, + /// Transient feedback for a keypress that had nothing to do (e.g. `a` + /// without `--guide`). Overrides the hunk-count/help hint on the right + /// while set; see `AppState::status_message`. + pub status_message: Option<&'a str>, + /// Whether the AI-generated Guide summary is still being computed for + /// the current diff — surfaced here so it's visible without toggling + /// into the Guide sidebar first. + pub guide_pending: bool, } /// Truncates a file path by abbreviating directory names to their first character. @@ -112,6 +120,11 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { frame.render_widget(footer, footer_area); } else { let watch_indicator = if data.watching { " watching" } else { "" }; + let guide_indicator = if data.guide_pending { + " guide: generating…" + } else { + "" + }; let max_filename_len = if data.search_state.has_query() { (data.area_width as usize).saturating_sub(80).min(40) } else { @@ -184,6 +197,10 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { Span::styled(viewed_indicator, Style::default().fg(t.ui.viewed).bg(bg)), ]; spans.extend(stats_spans); + spans.push(Span::styled( + guide_indicator, + Style::default().fg(t.ui.watching).bg(bg), + )); spans } else { // Normal diff mode: show commit reference @@ -203,11 +220,23 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { Span::styled(viewed_indicator, Style::default().fg(t.ui.viewed).bg(bg)), ]; spans.extend(stats_spans); - spans.push(Span::styled(watch_indicator, Style::default().fg(t.ui.watching).bg(bg))); + spans.push(Span::styled( + watch_indicator, + Style::default().fg(t.ui.watching).bg(bg), + )); + spans.push(Span::styled( + guide_indicator, + Style::default().fg(t.ui.watching).bg(bg), + )); spans }; - let right_spans: Vec = if data.search_state.has_query() { + let right_spans: Vec = if let Some(msg) = data.status_message { + vec![Span::styled( + format!(" {} ", msg), + Style::default().fg(t.ui.highlight).bg(bg), + )] + } else if data.search_state.has_query() { let match_count = data.search_state.match_count(); let current_idx = data .search_state @@ -223,10 +252,7 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { format!("[0/0] /{} ", data.search_state.query) }; vec![ - Span::styled( - search_info, - Style::default().fg(t.ui.highlight).bg(bg), - ), + Span::styled(search_info, Style::default().fg(t.ui.highlight).bg(bg)), Span::styled( " n/N navigate ", Style::default().fg(t.ui.text_muted).bg(bg), @@ -259,10 +285,7 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { }, Style::default().fg(t.ui.text_muted).bg(bg), ), - Span::styled( - " ? help ", - Style::default().fg(t.ui.text_muted).bg(bg), - ), + Span::styled(" ? help ", Style::default().fg(t.ui.text_muted).bg(bg)), ] }; @@ -277,10 +300,7 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { let padding = footer_width.saturating_sub(left_len + right_len); let mut final_spans: Vec = left_line.spans; - final_spans.push(Span::styled( - " ".repeat(padding), - Style::default().bg(bg), - )); + final_spans.push(Span::styled(" ".repeat(padding), Style::default().bg(bg))); final_spans.extend(right_line.spans); let footer = Paragraph::new(Line::from(final_spans)).style(Style::default().bg(bg)); diff --git a/src/command/diff/state.rs b/src/command/diff/state.rs index 8c6f80d5..43e8988f 100644 --- a/src/command/diff/state.rs +++ b/src/command/diff/state.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::time::SystemTime; +use std::time::{Instant, SystemTime}; use tree_sitter::{Parser, Tree}; @@ -263,8 +263,16 @@ pub struct AppState { pub guide_group_selected: usize, /// Selected file index within the currently selected Guide group. pub guide_file_selected: usize, + /// Transient feedback for a keypress that had nothing to do (e.g. `a` + /// without `--guide`), paired with when it was set so the main loop can + /// clear it after `STATUS_MESSAGE_TTL`. Shown in the footer. + pub status_message: Option<(String, Instant)>, } +/// How long a `status_message` stays visible in the footer before the main +/// loop clears it. +pub const STATUS_MESSAGE_TTL: std::time::Duration = std::time::Duration::from_secs(2); + fn compute_total_line_stats(file_diffs: &[FileDiff]) -> (usize, usize) { let mut added = 0usize; let mut removed = 0usize; @@ -373,6 +381,7 @@ impl AppState { sidebar_mode: SidebarMode::default(), guide_group_selected: 0, guide_file_selected: 0, + status_message: None, } } @@ -609,6 +618,23 @@ impl AppState { content_y - cumulative } + /// Set a transient footer message (e.g. feedback for a keypress that had + /// nothing to do). Cleared automatically by `expire_status_message` once + /// `STATUS_MESSAGE_TTL` has elapsed. + pub fn set_status_message(&mut self, message: impl Into) { + self.status_message = Some((message.into(), Instant::now())); + } + + /// Drop `status_message` once it has been visible for `STATUS_MESSAGE_TTL`. + /// Called once per main-loop iteration so the message fades on its own. + pub fn expire_status_message(&mut self) { + if let Some((_, set_at)) = &self.status_message { + if set_at.elapsed() >= STATUS_MESSAGE_TTL { + self.status_message = None; + } + } + } + /// Clear all selection state pub fn clear_selection(&mut self) { self.diff_panel_focus = DiffPanelFocus::None;