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 <louis@bloop.ai>
This commit is contained in:
@@ -8,7 +8,7 @@ use git2::{
|
|||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
use ts_rs::TS;
|
use ts_rs::TS;
|
||||||
use utils::diff::{Diff, DiffChangeKind, FileDiffDetails};
|
use utils::diff::{Diff, DiffChangeKind, FileDiffDetails, compute_line_change_counts};
|
||||||
|
|
||||||
mod cli;
|
mod cli;
|
||||||
|
|
||||||
@@ -517,16 +517,15 @@ impl GitService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If contents are omitted, try to compute line stats via libgit2 Patch
|
// Always compute line stats via libgit2 Patch
|
||||||
let mut additions: Option<usize> = None;
|
let (additions, deletions) = if let Ok(Some(patch)) =
|
||||||
let mut deletions: Option<usize> = None;
|
git2::Patch::from_diff(&diff, delta_index)
|
||||||
if content_omitted
|
|
||||||
&& let Ok(Some(patch)) = git2::Patch::from_diff(&diff, delta_index)
|
|
||||||
&& let Ok((_ctx, adds, dels)) = patch.line_stats()
|
&& let Ok((_ctx, adds, dels)) = patch.line_stats()
|
||||||
{
|
{
|
||||||
additions = Some(adds);
|
(Some(adds), Some(dels))
|
||||||
deletions = Some(dels);
|
} else {
|
||||||
}
|
(None, None)
|
||||||
|
};
|
||||||
|
|
||||||
file_diffs.push(Diff {
|
file_diffs.push(Diff {
|
||||||
change,
|
change,
|
||||||
@@ -732,6 +731,23 @@ impl GitService {
|
|||||||
change = DiffChangeKind::PermissionChange;
|
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 {
|
Diff {
|
||||||
change,
|
change,
|
||||||
old_path: old_path_opt,
|
old_path: old_path_opt,
|
||||||
@@ -739,8 +755,8 @@ impl GitService {
|
|||||||
old_content,
|
old_content,
|
||||||
new_content,
|
new_content,
|
||||||
content_omitted,
|
content_omitted,
|
||||||
additions: None,
|
additions,
|
||||||
deletions: None,
|
deletions,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ import {
|
|||||||
TooltipProvider,
|
TooltipProvider,
|
||||||
TooltipTrigger,
|
TooltipTrigger,
|
||||||
} from '@/components/ui/tooltip';
|
} from '@/components/ui/tooltip';
|
||||||
import type { TaskAttempt, Diff } from 'shared/types';
|
import type { TaskAttempt, Diff, DiffChangeKind } from 'shared/types';
|
||||||
import GitOperations, {
|
import GitOperations, {
|
||||||
type GitOperationsInputs,
|
type GitOperationsInputs,
|
||||||
} from '@/components/tasks/Toolbar/GitOperations.tsx';
|
} from '@/components/tasks/Toolbar/GitOperations.tsx';
|
||||||
@@ -24,59 +24,81 @@ interface DiffsPanelProps {
|
|||||||
gitOps?: GitOperationsInputs;
|
gitOps?: GitOperationsInputs;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type DiffCollapseDefaults = Record<DiffChangeKind, boolean>;
|
||||||
|
|
||||||
|
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) {
|
export function DiffsPanel({ selectedAttempt, gitOps }: DiffsPanelProps) {
|
||||||
const { t } = useTranslation('tasks');
|
const { t } = useTranslation('tasks');
|
||||||
const [loading, setLoading] = useState(true);
|
const [loadingState, setLoadingState] = useState<
|
||||||
|
'loading' | 'loaded' | 'timed-out'
|
||||||
|
>('loading');
|
||||||
const [collapsedIds, setCollapsedIds] = useState<Set<string>>(new Set());
|
const [collapsedIds, setCollapsedIds] = useState<Set<string>>(new Set());
|
||||||
const [hasInitialized, setHasInitialized] = useState(false);
|
const [processedIds, setProcessedIds] = useState<Set<string>>(new Set());
|
||||||
const { diffs, error } = useDiffStream(selectedAttempt?.id ?? null, true);
|
const { diffs, error } = useDiffStream(selectedAttempt?.id ?? null, true);
|
||||||
const { fileCount, added, deleted } = useDiffSummary(
|
const { fileCount, added, deleted } = useDiffSummary(
|
||||||
selectedAttempt?.id ?? null
|
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
|
// If no diffs arrive within 3 seconds, stop showing the spinner
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!loading) return;
|
if (loadingState !== 'loading') return;
|
||||||
const timer = setTimeout(() => {
|
const timer = setTimeout(() => setLoadingState('timed-out'), 3000);
|
||||||
if (diffs.length === 0) {
|
|
||||||
setLoading(false);
|
|
||||||
}
|
|
||||||
}, 3000);
|
|
||||||
return () => clearTimeout(timer);
|
return () => clearTimeout(timer);
|
||||||
}, [loading, diffs.length]);
|
}, [loadingState]);
|
||||||
|
|
||||||
// Default-collapse certain change kinds on first load only
|
if (diffs.length > 0 && loadingState === 'loading') {
|
||||||
useEffect(() => {
|
setLoadingState('loaded');
|
||||||
if (diffs.length === 0) return;
|
}
|
||||||
if (hasInitialized) return; // only run once per attempt
|
|
||||||
const kindsToCollapse = new Set([
|
if (diffs.length > 0) {
|
||||||
'deleted',
|
const newDiffs = diffs
|
||||||
'renamed',
|
.map((d, index) => ({ diff: d, index }))
|
||||||
'copied',
|
.filter((d) => {
|
||||||
'permissionChange',
|
const id = getDiffId(d);
|
||||||
]);
|
return !processedIds.has(id);
|
||||||
const initial = new Set(
|
});
|
||||||
diffs
|
|
||||||
.filter((d) => kindsToCollapse.has(d.change))
|
if (newDiffs.length > 0) {
|
||||||
.map((d, i) => d.newPath || d.oldPath || String(i))
|
const newIds = newDiffs.map(getDiffId);
|
||||||
);
|
const toCollapse = newDiffs
|
||||||
if (initial.size > 0) setCollapsedIds(initial);
|
.filter(
|
||||||
setHasInitialized(true);
|
({ diff }) =>
|
||||||
}, [diffs, hasInitialized]);
|
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(() => {
|
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]);
|
}, [diffs]);
|
||||||
|
|
||||||
const toggle = useCallback((id: string) => {
|
const toggle = useCallback((id: string) => {
|
||||||
|
|||||||
@@ -111,6 +111,7 @@ function DiffsPanelContainer({
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<DiffsPanel
|
<DiffsPanel
|
||||||
|
key={attempt?.id}
|
||||||
selectedAttempt={attempt}
|
selectedAttempt={attempt}
|
||||||
gitOps={
|
gitOps={
|
||||||
attempt && selectedTask
|
attempt && selectedTask
|
||||||
|
|||||||
Reference in New Issue
Block a user