From cd8e03668cf0a1e2aa91b012c05dbd1ecebd1b39 Mon Sep 17 00:00:00 2001 From: "Xinyao (Morry) Niu" Date: Wed, 22 Jul 2026 19:05:37 +0800 Subject: [PATCH] fix(search): preserve dynamic scan ordering Defer independently inserted older history until a forward search reaches the true source prefix, and prioritize newer append ranges during backward scans even after an earlier append was partially consumed. Fill bounded search windows across short ViewFile reads so reverse scans cannot consume different coordinates than they inspected. Add regressions for huge tail records, multi-append ordering, reset boundaries, short reads, and search-index rebuilds. --- .../src/viewer/file/input/search.rs | 62 +++++- .../fmtview-core/src/viewer/tests/search.rs | 190 ++++++++++++++++++ crates/fmtview-core/tests/record_timeline.rs | 67 ++++++ 3 files changed, 313 insertions(+), 6 deletions(-) diff --git a/crates/fmtview-core/src/viewer/file/input/search.rs b/crates/fmtview-core/src/viewer/file/input/search.rs index 6fc3e4a..ec08c6c 100644 --- a/crates/fmtview-core/src/viewer/file/input/search.rs +++ b/crates/fmtview-core/src/viewer/file/input/search.rs @@ -1,7 +1,7 @@ use std::collections::VecDeque; use crate::viewer::{KeyCode, KeyModifiers}; -use anyhow::Result; +use anyhow::{Result, bail}; use crate::load::ViewFile; @@ -103,6 +103,29 @@ impl SearchTask { if start >= end { return; } + if self.direction == SearchDirection::Backward { + let boundary = self + .spans + .iter() + .position(|span| span.kind != SearchSpanKind::Initial) + .unwrap_or(self.spans.len()); + if let Some(span) = self.spans.get_mut(boundary) + && span.kind == SearchSpanKind::Appended + && span.end == start + { + span.end = end; + return; + } + self.spans.insert( + boundary, + SearchSpan { + start, + end, + kind: SearchSpanKind::Appended, + }, + ); + return; + } if self.awaiting_older { let priority_end = self .spans @@ -495,9 +518,12 @@ pub(in crate::viewer) fn process_search_step( && file.has_older_records() && (task.awaiting_older || !task.has_work() - || task - .current_span() - .is_some_and(|span| span.kind == SearchSpanKind::Wrapped)) + || task.current_span().is_some_and(|span| { + matches!( + span.kind, + SearchSpanKind::Inserted | SearchSpanKind::Wrapped + ) + })) { task.awaiting_older = true; } else if !file.has_older_records() { @@ -640,7 +666,7 @@ pub(in crate::viewer) fn scan_search_forward( }); }; let count = span.len().min(SEARCH_CHUNK_LINES); - let lines = file.read_window(span.start, count)?; + let lines = read_search_window(file, span.start, count)?; for (offset, line) in lines.iter().enumerate() { if let Some(byte_index) = line.find(&task.query) { return Ok(SearchStep { @@ -671,7 +697,7 @@ pub(in crate::viewer) fn scan_search_backward( }; let count = span.len().min(SEARCH_CHUNK_LINES); let start = span.end.saturating_sub(count); - let lines = file.read_window(start, count)?; + let lines = read_search_window(file, start, count)?; for (offset, line) in lines.iter().enumerate().rev() { if let Some(byte_index) = line.rfind(&task.query) { return Ok(SearchStep { @@ -689,3 +715,27 @@ pub(in crate::viewer) fn scan_search_backward( scanned: lines.len(), }) } + +fn read_search_window(file: &dyn ViewFile, start: usize, count: usize) -> Result> { + let mut lines = Vec::with_capacity(count); + while lines.len() < count { + let remaining = count - lines.len(); + let Some(next_start) = start.checked_add(lines.len()) else { + bail!("search window start overflow"); + }; + let chunk = file.read_window(next_start, remaining)?; + if chunk.is_empty() { + bail!( + "view file returned an empty search window at line {next_start} with {remaining} lines remaining" + ); + } + if chunk.len() > remaining { + bail!( + "view file returned {} search lines for a {remaining}-line request", + chunk.len() + ); + } + lines.extend(chunk); + } + Ok(lines) +} diff --git a/crates/fmtview-core/src/viewer/tests/search.rs b/crates/fmtview-core/src/viewer/tests/search.rs index a71bc68..cc4242f 100644 --- a/crates/fmtview-core/src/viewer/tests/search.rs +++ b/crates/fmtview-core/src/viewer/tests/search.rs @@ -410,6 +410,103 @@ fn backward_search_targets_last_match_on_matching_line() { ); } +#[test] +fn backward_search_preserves_order_across_short_read_windows() { + struct ShortReadFile { + lines: Vec, + max_window: usize, + } + + impl ViewFile for ShortReadFile { + fn label(&self) -> &str { + "short-read" + } + + fn line_count(&self) -> usize { + self.lines.len() + } + + fn byte_len(&self) -> u64 { + 0 + } + + fn byte_offset_for_line(&self, _line: usize) -> u64 { + 0 + } + + fn read_window(&self, start: usize, count: usize) -> Result> { + Ok(self + .lines + .iter() + .skip(start) + .take(count.min(self.max_window)) + .cloned() + .collect()) + } + } + + let mut lines = (0..10) + .map(|line| format!("line-{line}")) + .collect::>(); + lines[2] = "needle-short-read-old".to_owned(); + lines[9] = "needle-short-read-new".to_owned(); + let file = ShortReadFile { + lines, + max_window: 3, + }; + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-short-read".to_owned(), + SearchDirection::Backward, + 9, + file.line_count(), + ); + + assert!(process_search_step(&file, &mut state).unwrap()); + assert_eq!(state.search_cursor, Some(9)); +} + +#[test] +fn search_rejects_an_empty_read_inside_a_reported_window() { + struct EmptyReadFile; + + impl ViewFile for EmptyReadFile { + fn label(&self) -> &str { + "empty-read" + } + + fn line_count(&self) -> usize { + 1 + } + + fn byte_len(&self) -> u64 { + 0 + } + + fn byte_offset_for_line(&self, _line: usize) -> u64 { + 0 + } + + fn read_window(&self, _start: usize, _count: usize) -> Result> { + Ok(Vec::new()) + } + } + + let file = EmptyReadFile; + let mut state = ViewState::default(); + start_search( + &mut state, + "needle".to_owned(), + SearchDirection::Forward, + 0, + file.line_count(), + ); + + let error = process_search_step(&file, &mut state).unwrap_err(); + assert!(error.to_string().contains("empty search window")); +} + #[test] fn search_reports_not_found_and_can_clear_message() { let file = indexed_lines(&["alpha", "beta"]); @@ -751,6 +848,13 @@ impl MutableSearchFile { (start, current.len()) } + fn insert(&self, at: usize, lines: impl IntoIterator) -> usize { + let inserted = lines.into_iter().collect::>(); + let count = inserted.len(); + self.lines.borrow_mut().splice(at..at, inserted); + count + } + fn read_counts(&self) -> Vec { let line_count = self.line_count(); let mut counts = vec![0_usize; line_count]; @@ -854,6 +958,61 @@ fn backward_search_does_not_rescan_wrapped_history_before_append_delta() { assert_eq!(counts[original_len + 1], 1); } +#[test] +fn backward_search_prioritizes_a_new_append_over_a_partially_scanned_append() { + let original_len = 8; + let file = MutableSearchFile::with_len(original_len); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-backward-multi-append".to_owned(), + SearchDirection::Backward, + 0, + original_len, + ); + + assert!(!process_search_step(&file, &mut state).unwrap()); + let first_append = std::iter::once("needle-backward-multi-append-old".to_owned()) + .chain( + (0..crate::viewer::file::SEARCH_CHUNK_LINES + 9) + .map(|line| format!("first-append-{line}")), + ) + .collect::>(); + let (first_start, first_end) = file.append(first_append); + state.extend_for_append(first_start, first_end); + + assert!(!process_search_step(&file, &mut state).unwrap()); + let (second_start, second_end) = file.append(["needle-backward-multi-append-new".to_owned()]); + state.extend_for_append(second_start, second_end); + assert!(process_search_step(&file, &mut state).unwrap()); + + assert_eq!(state.search_cursor, Some(second_end - 1)); +} + +#[test] +fn backward_search_visits_an_insert_at_the_current_span_end_before_that_span() { + let original_len = crate::viewer::file::SEARCH_CHUNK_LINES + 10; + let file = MutableSearchFile::with_len(original_len); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-backward-boundary-insert".to_owned(), + SearchDirection::Backward, + 0, + original_len, + ); + + assert!(!process_search_step(&file, &mut state).unwrap()); + assert!(!process_search_step(&file, &mut state).unwrap()); + let at = 10; + file.lines.borrow_mut()[at - 1] = "needle-backward-boundary-insert-old".to_owned(); + let inserted = file.insert(at, ["needle-backward-boundary-insert-new".to_owned()]); + state.shift_for_insert(at, inserted); + assert!(process_search_step(&file, &mut state).unwrap()); + + assert_eq!(state.search_cursor, Some(at)); +} + #[test] fn append_invalidates_and_recounts_an_exact_search_index() { let file = MutableSearchFile::with_len(3); @@ -876,3 +1035,34 @@ fn append_invalidates_and_recounts_an_exact_search_index() { assert!(state.search_index.as_ref().unwrap().exact); assert_eq!(state.search_index.as_ref().unwrap().matches, 1); } + +#[test] +fn insert_rebuilds_the_search_index_and_shifted_match_ordinal() { + let file = MutableSearchFile::with_len(3); + file.lines.borrow_mut()[1] = "needle-index-original".to_owned(); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-index".to_owned(), + SearchDirection::Forward, + 0, + file.line_count(), + ); + assert!(process_search_step(&file, &mut state).unwrap()); + assert!(process_search_index_step(&file, &mut state).unwrap()); + assert_eq!(state.search_match_ordinal, Some(1)); + + let inserted = file.insert(0, ["needle-index needle-index".to_owned()]); + state.shift_for_insert(0, inserted); + let index = state.search_index.as_ref().unwrap(); + assert_eq!(index.counted_lines, 0); + assert_eq!(index.matches, 0); + assert!(!index.exact); + + assert!(process_search_index_step(&file, &mut state).unwrap()); + let index = state.search_index.as_ref().unwrap(); + assert_eq!(index.matches, 3); + assert_eq!(index.counted_lines, 4); + assert!(index.exact); + assert_eq!(state.search_match_ordinal, Some(3)); +} diff --git a/crates/fmtview-core/tests/record_timeline.rs b/crates/fmtview-core/tests/record_timeline.rs index de9c256..0d49ac9 100644 --- a/crates/fmtview-core/tests/record_timeline.rs +++ b/crates/fmtview-core/tests/record_timeline.rs @@ -995,6 +995,41 @@ fn wrapped_forward_search_waits_for_the_true_prefix_before_choosing_a_match() { assert!(!wrapped.contains("needle-newer-prefix-batch"), "{wrapped}"); } +#[test] +fn forward_search_from_loaded_start_waits_for_true_prefix_after_a_huge_tail_record() { + let huge_tail = format!( + "{{\"items\":[{}]}}\n", + std::iter::repeat_n("0", 5_000) + .collect::>() + .join(",") + ) + .into_bytes(); + let (_handle, timeline) = fake_timeline((0..1_000).map(|index| match index { + 0 => b"{\"message\":\"needle-oldest\"}\n".to_vec(), + 950 => b"{\"message\":\"needle-newer-prefix-batch\"}\n".to_vec(), + 999 => huge_tail.clone(), + _ => record(index), + })); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(2, 64 * 1024), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 10); + viewer.render(size, None).unwrap(); + viewer.handle_event(key(KeyCode::Home), FileViewer::page_for_size(size)); + viewer.render(size, None).unwrap(); + + enter_search(&mut viewer, size, "needle"); + advance_until_idle(&mut viewer); + let found = frame_text(viewer.render(size, None).unwrap()); + + assert!(found.contains("needle-oldest"), "{found}"); + assert!(!found.contains("needle-newer-prefix-batch"), "{found}"); +} + #[test] fn active_forward_search_includes_newer_records_arriving_at_the_boundary() { let (handle, timeline) = fake_timeline([record(0), record(1)]); @@ -1079,6 +1114,38 @@ fn active_search_crosses_a_reset_tail_and_lazily_inserted_older_records() { assert!(text.contains("needle-in-reset-prefix"), "{footer}"); } +#[test] +fn backward_search_orders_a_reset_tail_before_its_lazily_inserted_prefix() { + let (handle, timeline) = fake_timeline((0..20).map(record)); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(32, 64 * 1024), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 8); + viewer.render(size, None).unwrap(); + viewer.handle_event(key(KeyCode::Home), FileViewer::page_for_size(size)); + enter_search(&mut viewer, size, "needle-reset-backward"); + viewer.handle_event(key(KeyCode::Char('N')), FileViewer::page_for_size(size)); + + handle.replace((0..140).map(|index| match index { + 0 => b"{\"message\":\"needle-reset-backward-prefix\"}\n".to_vec(), + 139 => b"{\"message\":\"needle-reset-backward-tail\"}\n".to_vec(), + _ => format!("{{\"message\":\"replacement-{index}\"}}\n").into_bytes(), + })); + viewer.preload().unwrap(); + advance_until_idle(&mut viewer); + let tail = frame_text(viewer.render(size, None).unwrap()); + assert!(tail.contains("needle-reset-backward-tail"), "{tail}"); + + viewer.handle_event(key(KeyCode::Char('N')), FileViewer::page_for_size(size)); + advance_until_idle(&mut viewer); + let prefix = frame_text(viewer.render(size, None).unwrap()); + assert!(prefix.contains("needle-reset-backward-prefix"), "{prefix}"); +} + #[test] fn repeated_search_finds_appended_match_after_complete_miss_in_each_follow_state() { for (mode, expected_follow) in [