hl-valdemar/vamp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Wiki Activity

perf: bound diff label projection during caret scrolling

Browse Source 
The label lag came from repeatedly projecting full viewports while walking backwards from the caret. Skip only candidates whose maximum possible source/wrap and label height cannot reach the target, then retain the original first-target search. Reuse already resolved scope contexts. The conservative bound preserves non-monotonic unified declaration visibility and independent sticky labels. Added regressions for split declaration-visibility ties and unified height changes; architecture updated. Verification: 10,176 differential projections matched; all 964 release tests pass; release binary built. Reduced repro p95: 3.9572ms -> 1.4467ms. Live 160x100 function-dense 360-key burst median catch-up: 22.895ms -> 9.611ms, with final cursor/viewport asserted. Full and no-dependency Clippy checks are blocked by existing repository warnings, including vendored Crossterm, blame, LSP, Markdown, syntax and existing TUI code. Temporary harnesses removed. No remote changes.
This commit is contained in:
Valdemar Lorenzen
2026-10-16 09:26:05 +02:00
parent 1df9e66cd5
commit 60a12110fa
3 changed files with 133 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
TODO.md
+1
View File
@@ -10,6 +10,7 @@
- each side uses its own syntax tree; scope changes within a hunk get separate labels
- [x] diff view: annotate folder with checkmark if all items under given folder has been checked AND the folder is collapsed.
- [x] diff view: automatically detect changes and update the view when relevant bookmarks move change ids (regards bookmark-based diff view; i.e., user selected at least one bookmark in the before/after picker)
- [ ] action to scroll viewport without having to move the cursor (like vim's `ctrl+e|u`).
- [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc.
- [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding.
- design question: would this require a server that the agent can interact with, or would local review files be fine? don't want to use server client stuff, if it would be easier and better to just use local files.
docs/architecture.md
+1 -1
View File
@@ -105,7 +105,7 @@ Each buffer has a frontend-owned parser/tree cache. Unchanged revision/path skip
Diff highlighting lazily owns two instances of the same syntax cache, keyed by the accepted comparison request ID and selected file. Before detects language from the old path when renamed; After uses the new path. Scrolling, focus changes, and explorer toggling reuse parsed spans without copying/reparsing snapshots. A fresh comparison identity prevents stale Working-checkout colors even when labels and paths are unchanged. Closing the view releases snapshot caches. Syntax styles patch foreground/modifiers over diff backgrounds; historical panes never receive live-buffer semantic tokens.
Scope context is a lazily built row index over the existing primary/injected syntax trees, invalidated by source or grammar changes and warmed by diff updates. Ordinary buffer highlighting does not build it. The frontend's visual-row projection inserts inline/sticky scope labels in wrapped and unwrapped diffs; source alignment and excerpt ranges contain no label rows. Coloring, native selection/caret overlays, and source navigation skip these presentation rows. Before/After labels use their independent trees rather than live-workspace symbol analysis.
Cached scope metadata records the declaration identifier's source point separately from its extent, including decorated definitions. The projection checks that point against each pane's actual source rows and column/wrap ranges, suppressing redundant labels and refilling reclaimed rows. Caret anchoring uses the resulting visual rows rather than a fixed label-height estimate.
Cached scope metadata records the declaration identifier's source point separately from its extent, including decorated definitions. The projection checks that point against each pane's actual source rows and column/wrap ranges, suppressing redundant labels and refilling reclaimed rows. Caret anchoring skips source/wrap positions only while their maximum possible label height cannot reach the target, then projects the remaining candidates. This retains the first matching anchor even when declaration visibility changes label height non-monotonically.
## Clipboard providers
src/tui.rs
+131 -15
View File
@@ -4588,7 +4588,7 @@ impl DiffWrapViewport {
let mut sticky = [true; 2];
while self.rows.len() < height {
let contexts = syntax.scopes(diff, position.display);
for mut scope in syntax.labels(diff, position, sticky) {
for mut scope in syntax.labels(diff, position, contexts, sticky) {
for side in 0..2 {
scope[side] &= !contexts[side].is_some_and(|context| {
self.visible_names[side].contains(&context.name)
@@ -4667,6 +4667,12 @@ impl DiffWrapViewport {
})
}
fn screen_row(&self, position: DiffWrapPosition) -> Option<usize> {
self.rows
.iter()
.position(|row| row.scope == [false; 2] && row.position == position)
}
fn update(
&mut self,
diff: &DiffState,
@@ -4721,11 +4727,7 @@ impl DiffWrapViewport {
&& let Some(position) = self.caret_position(diff, file, focus, cursor)
{
let margin = scroll_margin(height, distance);
let row = self
.rows
.iter()
.take(height)
.position(|row| row.scope == [false; 2] && row.position == position);
let row = self.screen_row(position).filter(|&row| row < height);
let anchor = anchor_row.or_else(|| {
if height == 0 {
None
@@ -4743,11 +4745,31 @@ impl DiffWrapViewport {
if let Some(row) = anchor {
self.top = position;
let target = row.min(height.saturating_sub(1));
// Skip only positions guaranteed to precede the target, even if every
// potential label is shown. Each side can also add one sticky label.
let mut upper_bound = 2;
let mut candidate = position;
loop {
let contexts = syntax.scopes(diff, candidate.display);
upper_bound += syntax
.labels(diff, candidate, contexts, [false; 2])
.into_iter()
.filter(|scope| scope[0] || scope[1])
.count();
if upper_bound >= target {
break;
}
self.top = candidate;
let previous = self.step(diff, file, candidate, false);
if previous == candidate {
break;
}
candidate = previous;
upper_bound += 1;
}
self.fill(diff, file, paint_height, syntax);
while self
.rows
.iter()
.position(|row| row.scope == [false; 2] && row.position == position)
.screen_row(position)
.is_some_and(|screen| screen < target)
{
let previous = self.step(diff, file, self.top, false);
@@ -4757,11 +4779,10 @@ impl DiffWrapViewport {
let top = self.top;
self.top = previous;
self.fill(diff, file, paint_height, syntax);
let screen = self
.rows
.iter()
.position(|row| row.scope == [false; 2] && row.position == position);
if screen.is_none_or(|screen| screen > target) {
if self
.screen_row(position)
.is_none_or(|screen| screen > target)
{
self.top = top;
self.fill(diff, file, paint_height, syntax);
break;
@@ -4834,9 +4855,9 @@ impl DiffSyntaxCache {
&self,
diff: &DiffState,
position: DiffWrapPosition,
scopes: [Option<crate::syntax::ScopeContext<'_>>; 2],
sticky: [bool; 2],
) -> [[bool; 2]; 2] {
let scopes = self.scopes(diff, position.display);
let show = [Focus::Before, Focus::After].map(|focus| {
let side = usize::from(focus == Focus::After);
if scopes[side].is_none() || (!sticky[side] && position.continuation > 0) {
@@ -8002,6 +8023,101 @@ mod tests {
}
}
#[test]
fn diff_scope_anchor_keeps_nearest_top_when_declarations_reclaim_rows() {
let before = (0..4)
.map(|index| {
format!(
"impl Container{index} {{\n fn run(&self) {{\n old_call();\n keep();\n }}\n}}\n"
)
})
.collect::<String>();
let after = before
.replace("Container", "Renamed")
.replace("old_call", "new_call");
let mut diff = diff_comparison(vec![syntax_diff_file("scope.rs", &before, &after)]);
diff.focus = Focus::After;
let file = &diff.comparison.as_ref().unwrap().files[0];
let areas = split_rect(pane_rect(Rect::new(0, 0, 120, 35)), SplitAxis::Horizontal)
.map(terminal_rect);
let mut syntax = DiffSyntaxCache::default();
syntax.update(
diff.comparison_id.unwrap(),
0,
file,
&file.after,
None,
None,
);
let mut viewport = DiffWrapViewport::default();
let cursor = Cursor { row: 15, column: 7 };
viewport.update(
&diff,
file,
areas,
Some((Focus::After, cursor)),
Some(15),
Some(0),
&syntax,
);
let position = viewport
.caret_position(&diff, file, Focus::After, cursor)
.unwrap();
assert_eq!(viewport.screen_row(position), Some(15));
assert_eq!(
diff.display_source_row(Focus::After, viewport.top.display),
Some(2),
"Keep the nearest enclosing function's sticky context, not earlier declarations"
);
assert_eq!(viewport.rows[0].scope, [true; 2]);
}
#[test]
fn diff_scope_anchor_stops_at_first_target_across_unified_visibility_changes() {
let before = (0..12)
.map(|index| {
format!(
"impl Container{index} {{\n fn run(&self) {{\n old_call();\n keep();\n }}\n}}\n"
)
})
.collect::<String>();
let after = before
.replace("Container", "Renamed")
.replace("old_call", "new_call");
let mut file = syntax_diff_file("scope.rs", &before, &after);
file.hunks = crate::diff::line_hunks(&file.before, false, &file.after, false);
let mut diff = diff_comparison(vec![file]);
diff.set_context_options(3, 15);
diff.set_view_mode(ViewMode::Unified);
diff.focus = Focus::Before;
let file = &diff.comparison.as_ref().unwrap().files[0];
let mut syntax = DiffSyntaxCache::default();
syntax.update(
diff.comparison_id.unwrap(),
0,
file,
&file.after,
None,
None,
);
let mut viewport = DiffWrapViewport::default();
viewport.update(
&diff,
file,
[Rect::new(0, 0, 120, 35); 2],
Some((Focus::Before, Cursor { row: 12, column: 7 })),
Some(16),
Some(0),
&syntax,
);
assert_eq!(
diff.display_source_row(Focus::After, viewport.top.display),
Some(2),
"Do not move past the first anchor when an earlier declaration hides two labels"
);
assert_eq!(viewport.rows[0].scope, [false, true]);
}
#[test]
fn diff_scope_labels_follow_declaration_visibility() {
let before = "@decorate()\nclass PolicyActiveStatusEnum(BaseEnum):\n ACTIVE = 1\n EXPIRED = 2\n FUTURE = 3\n UNKNOWN = 4\n";