From adf759fbbeb05c8587256d47c528a7e8e249115c Mon Sep 17 00:00:00 2001 From: Cal Courtney Date: Thu, 18 Dec 2025 09:41:53 +0000 Subject: [PATCH] feat: add configurable auto-collapse for large diffs (#1587) * perf: remove unecessary useEffects * feat: add settings for default diff type collapsing * feat: add setting for max line default collapse * refactor: use object for default collapse config storage * fix: use diff changes for max line count * refactor: use consistent ids as fallback for id-less diffs * chore: ran formatter * chore: revert configurability and use fe defaults for auto-collapse * Debug None additions/deletions for diff (vibe-kanban 72a2a541) crates/utils/src/diff.rs /api/task-attempts/.../diff/ws returns diffs with "additions": null", "deletions": null * use backend additions/deletions --------- Co-authored-by: Louis Knight-Webb --- crates/services/src/services/git.rs | 38 +++++-- frontend/src/components/panels/DiffsPanel.tsx | 102 +++++++++++------- frontend/src/pages/ProjectTasks.tsx | 1 + 3 files changed, 90 insertions(+), 51 deletions(-) diff --git a/crates/services/src/services/git.rs b/crates/services/src/services/git.rs index d3db0b8f..d0018f24 100644 --- a/crates/services/src/services/git.rs +++ b/crates/services/src/services/git.rs @@ -8,7 +8,7 @@ use git2::{ use serde::{Deserialize, Serialize}; use thiserror::Error; use ts_rs::TS; -use utils::diff::{Diff, DiffChangeKind, FileDiffDetails}; +use utils::diff::{Diff, DiffChangeKind, FileDiffDetails, compute_line_change_counts}; mod cli; @@ -517,16 +517,15 @@ impl GitService { } } - // If contents are omitted, try to compute line stats via libgit2 Patch - let mut additions: Option = None; - let mut deletions: Option = None; - if content_omitted - && let Ok(Some(patch)) = git2::Patch::from_diff(&diff, delta_index) + // Always compute line stats via libgit2 Patch + let (additions, deletions) = if let Ok(Some(patch)) = + git2::Patch::from_diff(&diff, delta_index) && let Ok((_ctx, adds, dels)) = patch.line_stats() { - additions = Some(adds); - deletions = Some(dels); - } + (Some(adds), Some(dels)) + } else { + (None, None) + }; file_diffs.push(Diff { change, @@ -732,6 +731,23 @@ impl GitService { change = DiffChangeKind::PermissionChange; } + // Compute line stats from available content + let (additions, deletions) = match (&old_content, &new_content) { + (Some(old), Some(new)) => { + let (adds, dels) = compute_line_change_counts(old, new); + (Some(adds), Some(dels)) + } + (Some(old), None) => { + // File deleted - all lines are deletions + (Some(0), Some(old.lines().count())) + } + (None, Some(new)) => { + // File added - all lines are additions + (Some(new.lines().count()), Some(0)) + } + (None, None) => (None, None), + }; + Diff { change, old_path: old_path_opt, @@ -739,8 +755,8 @@ impl GitService { old_content, new_content, content_omitted, - additions: None, - deletions: None, + additions, + deletions, } } diff --git a/frontend/src/components/panels/DiffsPanel.tsx b/frontend/src/components/panels/DiffsPanel.tsx index fbd138ef..54fd87e1 100644 --- a/frontend/src/components/panels/DiffsPanel.tsx +++ b/frontend/src/components/panels/DiffsPanel.tsx @@ -14,7 +14,7 @@ import { TooltipProvider, TooltipTrigger, } from '@/components/ui/tooltip'; -import type { TaskAttempt, Diff } from 'shared/types'; +import type { TaskAttempt, Diff, DiffChangeKind } from 'shared/types'; import GitOperations, { type GitOperationsInputs, } from '@/components/tasks/Toolbar/GitOperations.tsx'; @@ -24,59 +24,81 @@ interface DiffsPanelProps { gitOps?: GitOperationsInputs; } +type DiffCollapseDefaults = Record; + +const DEFAULT_DIFF_COLLAPSE_DEFAULTS: DiffCollapseDefaults = { + added: false, + deleted: true, + modified: false, + renamed: true, + copied: true, + permissionChange: true, +}; + +const DEFAULT_COLLAPSE_MAX_LINES = 200; + +const exceedsMaxLineCount = (d: Diff, maxLines: number): boolean => { + if (d.additions != null || d.deletions != null) + return (d.additions ?? 0) + (d.deletions ?? 0) > maxLines; + + return true; +}; + +const getDiffId = ({ diff, index }: { diff: Diff; index: number }) => + `${diff.newPath || diff.oldPath || index}`; + export function DiffsPanel({ selectedAttempt, gitOps }: DiffsPanelProps) { const { t } = useTranslation('tasks'); - const [loading, setLoading] = useState(true); + const [loadingState, setLoadingState] = useState< + 'loading' | 'loaded' | 'timed-out' + >('loading'); const [collapsedIds, setCollapsedIds] = useState>(new Set()); - const [hasInitialized, setHasInitialized] = useState(false); + const [processedIds, setProcessedIds] = useState>(new Set()); const { diffs, error } = useDiffStream(selectedAttempt?.id ?? null, true); const { fileCount, added, deleted } = useDiffSummary( selectedAttempt?.id ?? null ); - useEffect(() => { - setLoading(true); - setHasInitialized(false); - }, [selectedAttempt?.id]); - - useEffect(() => { - if (diffs.length > 0 && loading) { - setLoading(false); - } - }, [diffs, loading]); - // If no diffs arrive within 3 seconds, stop showing the spinner useEffect(() => { - if (!loading) return; - const timer = setTimeout(() => { - if (diffs.length === 0) { - setLoading(false); - } - }, 3000); + if (loadingState !== 'loading') return; + const timer = setTimeout(() => setLoadingState('timed-out'), 3000); return () => clearTimeout(timer); - }, [loading, diffs.length]); + }, [loadingState]); - // Default-collapse certain change kinds on first load only - useEffect(() => { - if (diffs.length === 0) return; - if (hasInitialized) return; // only run once per attempt - const kindsToCollapse = new Set([ - 'deleted', - 'renamed', - 'copied', - 'permissionChange', - ]); - const initial = new Set( - diffs - .filter((d) => kindsToCollapse.has(d.change)) - .map((d, i) => d.newPath || d.oldPath || String(i)) - ); - if (initial.size > 0) setCollapsedIds(initial); - setHasInitialized(true); - }, [diffs, hasInitialized]); + if (diffs.length > 0 && loadingState === 'loading') { + setLoadingState('loaded'); + } + + if (diffs.length > 0) { + const newDiffs = diffs + .map((d, index) => ({ diff: d, index })) + .filter((d) => { + const id = getDiffId(d); + return !processedIds.has(id); + }); + + if (newDiffs.length > 0) { + const newIds = newDiffs.map(getDiffId); + const toCollapse = newDiffs + .filter( + ({ diff }) => + DEFAULT_DIFF_COLLAPSE_DEFAULTS[diff.change] || + exceedsMaxLineCount(diff, DEFAULT_COLLAPSE_MAX_LINES) + ) + .map(getDiffId); + + setProcessedIds((prev) => new Set([...prev, ...newIds])); + if (toCollapse.length > 0) { + setCollapsedIds((prev) => new Set([...prev, ...toCollapse])); + } + } + } + + const loading = loadingState === 'loading'; const ids = useMemo(() => { - return diffs.map((d, i) => d.newPath || d.oldPath || String(i)); + return diffs.map((d, i) => getDiffId({ diff: d, index: i })); }, [diffs]); const toggle = useCallback((id: string) => { diff --git a/frontend/src/pages/ProjectTasks.tsx b/frontend/src/pages/ProjectTasks.tsx index 090b31af..8e1366a6 100644 --- a/frontend/src/pages/ProjectTasks.tsx +++ b/frontend/src/pages/ProjectTasks.tsx @@ -111,6 +111,7 @@ function DiffsPanelContainer({ return (