From c650e5c54c7aa446801ccc0e6fbecde0e82ebad3 Mon Sep 17 00:00:00 2001 From: "Xinyao (Morry) Niu" Date: Wed, 22 Jul 2026 18:52:36 +0800 Subject: [PATCH 1/3] fix(search): track appended scan ranges Represent active searches as disjoint half-open spans so live appends and lazy older inserts extend only unvisited ranges. Preserve true-prefix ordering while allowing committed appends to stay searchable during follow. Add forward/backward wrap, reset-tail, completed-miss, index recount, and lazy-history regressions. Clarify committed-boundary discovery separately from the bounded initial tail load. --- CHANGELOG.md | 5 + README.md | 25 +- crates/fmtview-core/src/viewer/file.rs | 5 + .../src/viewer/file/input/search.rs | 392 +++++++++++++----- .../src/viewer/file/input/state.rs | 17 +- .../fmtview-core/src/viewer/tests/search.rs | 148 +++++++ crates/fmtview-core/tests/record_timeline.rs | 174 ++++++++ docs/architecture.md | 11 +- docs/performance.md | 8 +- 9 files changed, 666 insertions(+), 119 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 424162d..bd6dc02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ for GitHub Release notes, so every published version must have a matching showing follow controls, while existing follow timelines retain append, detach, pause, and reset behavior. +### Fixed + +- Keep forward and backward search sessions aware of committed records appended + after the search began, including when repeating a completed miss. + ## [0.6.0] - 2026-07-22 ### Added diff --git a/README.md b/README.md index 41c4d74..b2abf5e 100644 --- a/README.md +++ b/README.md @@ -274,10 +274,14 @@ committed tail: fmtview --follow service.jsonl ``` -The first frame reverse-scans only enough source data to build the last -viewport; it does not format or index the file from the beginning. A final -record is committed only after its newline arrives, so a writer's partial EOF -record is never displayed early. While the viewport remains at the bottom, +The first frame reverse-scans from EOF to the newest committed record boundary; +that discovery work is proportional to the incomplete EOF suffix, so a +newline-free file is the intentional worst case. It then loads the initial +committed tail under a separate 128-record/4 MiB budget; one record may exceed +the byte budget so it can remain intact. Earlier committed history is not +formatted or indexed from the beginning. A final record is committed only after +its newline arrives, so a writer's partial EOF record is never displayed early. +While the viewport remains at the bottom, committed appends advance it automatically. Scrolling up changes the footer to `follow:detached`; scrolling back to the physical bottom reattaches. Press `f` to pause or resume follow explicitly. Search plus structure, chat-role, and tool @@ -286,7 +290,11 @@ after opening. Follow mode requires a real file and an interactive stdout terminal. Ordinary viewing and redirected output are unchanged; `--follow` never turns redirected -stdout into an endless stream. +stdout into an endless stream. The built-in file source is intended for +append-oriented logs and detects normal truncation, rotation, replacement, and +copytruncate signals. A producer that performs arbitrary in-place rewrites +without a distinguishable identity or metadata change should provide its own +`RecordTimeline` implementation. ## Embedding a Record Timeline @@ -617,9 +625,12 @@ rendered output in memory for browsing. - Record-like TTY previews, such as JSONL logs, use a lazy path: `fmtview` sniffs a small prefix to confirm that the input is independent records, then formats only the records needed for the visible window. -- Follow-mode JSONL opens from a bounded reverse tail scan. Refresh locates the +- Follow-mode JSONL opens from a chunked reverse tail scan. Refresh locates the newest committed delimiter from EOF, and record/byte budgets bound subsequent - older and newer loads; no persistent full-file index is required. + older and newer loads; no persistent full-file index is required. Committed + boundary discovery work is proportional to the incomplete EOF suffix. The + separate initial committed-tail load is capped at 128 records and 4 MiB, + except that one record may exceed the byte budget. - Lazy preview writes transformed records into a temporary spool and keeps compact offsets, not formatted strings, in memory. The title shows `N+` lines while the session index is still incomplete and idle time extends the index. diff --git a/crates/fmtview-core/src/viewer/file.rs b/crates/fmtview-core/src/viewer/file.rs index f1b06cb..bc586f1 100644 --- a/crates/fmtview-core/src/viewer/file.rs +++ b/crates/fmtview-core/src/viewer/file.rs @@ -420,6 +420,11 @@ impl FileViewer { self.state .shift_for_insert(change.inserted_at, change.inserted_lines); } + if change.appended_lines > 0 { + let end = self.file.line_count(); + self.state + .extend_for_append(end.saturating_sub(change.appended_lines), end); + } if change.appended_lines > 0 && self.state.follow == Some(FollowState::Following) { set_file_end(&mut self.state, self.file.line_count()); } diff --git a/crates/fmtview-core/src/viewer/file/input/search.rs b/crates/fmtview-core/src/viewer/file/input/search.rs index f3ee3ac..6fc3e4a 100644 --- a/crates/fmtview-core/src/viewer/file/input/search.rs +++ b/crates/fmtview-core/src/viewer/file/input/search.rs @@ -1,3 +1,5 @@ +use std::collections::VecDeque; + use crate::viewer::{KeyCode, KeyModifiers}; use anyhow::Result; @@ -19,11 +21,230 @@ pub(in crate::viewer) struct SearchTarget { pub(in crate::viewer) struct SearchTask { pub(in crate::viewer) query: String, pub(in crate::viewer) direction: SearchDirection, - pub(in crate::viewer) next_line: usize, - pub(in crate::viewer) remaining: usize, + // Disjoint half-open ranges of currently loaded lines not yet visited by + // this search. The front span is consumed in `direction`; appended and + // inserted ranges are queued without restarting the completed prefix. + spans: VecDeque, + known_line_count: usize, + // A forward wrap freezes non-append spans until the true older boundary is + // known. Live append spans remain eligible while that prefix is loading. pub(in crate::viewer) awaiting_older: bool, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct SearchSpan { + start: usize, + end: usize, + kind: SearchSpanKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SearchSpanKind { + Initial, + Wrapped, + Appended, + Inserted, +} + +impl SearchSpan { + fn new(start: usize, end: usize, kind: SearchSpanKind) -> Option { + (start < end).then_some(Self { start, end, kind }) + } + + fn len(self) -> usize { + self.end.saturating_sub(self.start) + } +} + +impl SearchTask { + fn new( + query: String, + direction: SearchDirection, + start_line: usize, + line_count: usize, + ) -> Self { + let mut spans = VecDeque::with_capacity(3); + let split = start_line.min(line_count.saturating_sub(1)); + match direction { + SearchDirection::Forward => { + push_span( + &mut spans, + SearchSpan::new(split, line_count, SearchSpanKind::Initial), + ); + push_span( + &mut spans, + SearchSpan::new(0, split, SearchSpanKind::Wrapped), + ); + } + SearchDirection::Backward => { + let split = split.saturating_add(1).min(line_count); + push_span( + &mut spans, + SearchSpan::new(0, split, SearchSpanKind::Initial), + ); + push_span( + &mut spans, + SearchSpan::new(split, line_count, SearchSpanKind::Wrapped), + ); + } + } + Self { + query, + direction, + spans, + known_line_count: line_count, + awaiting_older: false, + } + } + + pub(in crate::viewer) fn extend_for_append(&mut self, start: usize, end: usize) { + let start = start.max(self.known_line_count); + self.known_line_count = self.known_line_count.max(end); + if start >= end { + return; + } + if self.awaiting_older { + let priority_end = self + .spans + .iter() + .position(|span| span.kind != SearchSpanKind::Appended) + .unwrap_or(self.spans.len()); + if priority_end > 0 && self.spans[priority_end - 1].end == start { + self.spans[priority_end - 1].end = end; + return; + } + self.spans.insert( + priority_end, + SearchSpan { + start, + end, + kind: SearchSpanKind::Appended, + }, + ); + return; + } + let deferred_boundary = self.spans.iter().position(|span| match self.direction { + SearchDirection::Forward => matches!( + span.kind, + SearchSpanKind::Inserted | SearchSpanKind::Wrapped + ), + SearchDirection::Backward => span.kind == SearchSpanKind::Wrapped, + }); + if let Some(boundary) = deferred_boundary { + if boundary > 0 + && self.spans[boundary - 1].kind == SearchSpanKind::Appended + && self.spans[boundary - 1].end == start + { + self.spans[boundary - 1].end = end; + return; + } + self.spans.insert( + boundary, + SearchSpan { + start, + end, + kind: SearchSpanKind::Appended, + }, + ); + return; + } + if let Some(last) = self.spans.back_mut() + && last.kind == SearchSpanKind::Appended + && last.end == start + { + last.end = end; + return; + } + self.spans.push_back(SearchSpan { + start, + end, + kind: SearchSpanKind::Appended, + }); + } + + pub(in crate::viewer) fn shift_for_insert(&mut self, at: usize, lines: usize) { + if lines == 0 { + return; + } + let mut absorbed = false; + for span in &mut self.spans { + if span.start < at && at < span.end { + span.end = span.end.saturating_add(lines); + absorbed = true; + } else if span.start >= at { + span.start = span.start.saturating_add(lines); + span.end = span.end.saturating_add(lines); + } + } + self.known_line_count = self.known_line_count.saturating_add(lines); + if absorbed { + return; + } + + let shifted_start = at.saturating_add(lines); + if let Some(existing) = self + .spans + .iter_mut() + .find(|span| span.kind != SearchSpanKind::Appended && span.start == shifted_start) + { + existing.start = at; + return; + } + let insert_at = self + .spans + .iter() + .position(|span| span.kind == SearchSpanKind::Wrapped) + .unwrap_or(self.spans.len()); + self.spans.insert( + insert_at, + SearchSpan { + start: at, + end: shifted_start, + kind: SearchSpanKind::Inserted, + }, + ); + } + + fn observe_line_count(&mut self, line_count: usize) { + if line_count > self.known_line_count { + self.extend_for_append(self.known_line_count, line_count); + } + } + + fn current_span(&self) -> Option { + self.spans.front().copied() + } + + fn consume(&mut self, lines: usize) { + let Some(span) = self.spans.front_mut() else { + return; + }; + let lines = lines.min(span.len()); + match self.direction { + SearchDirection::Forward => span.start = span.start.saturating_add(lines), + SearchDirection::Backward => span.end = span.end.saturating_sub(lines), + } + if span.start == span.end { + self.spans.pop_front(); + } + } + + fn has_work(&self) -> bool { + !self.spans.is_empty() + } + + #[cfg(test)] + pub(in crate::viewer) fn span_count(&self) -> usize { + self.spans.len() + } +} + +fn push_span(spans: &mut VecDeque, span: Option) { + if let Some(span) = span { + spans.push_back(span); + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(in crate::viewer) struct SearchMatchIndex { pub(in crate::viewer) query: String, @@ -191,13 +412,7 @@ pub(in crate::viewer) fn start_search( return true; } - state.search_task = Some(SearchTask { - query, - direction, - next_line: start_line.min(line_count.saturating_sub(1)), - remaining: line_count, - awaiting_older: false, - }); + state.search_task = Some(SearchTask::new(query, direction, start_line, line_count)); true } @@ -275,14 +490,43 @@ pub(in crate::viewer) fn process_search_step( return Ok(false); }; - if task.awaiting_older { + task.observe_line_count(file.line_count()); + if task.direction == SearchDirection::Forward + && file.has_older_records() + && (task.awaiting_older + || !task.has_work() + || task + .current_span() + .is_some_and(|span| span.kind == SearchSpanKind::Wrapped)) + { + task.awaiting_older = true; + } else if !file.has_older_records() { + task.awaiting_older = false; + } + + if task.awaiting_older + && file.has_older_records() + && !task + .current_span() + .is_some_and(|span| span.kind == SearchSpanKind::Appended) + { + state.search_task = Some(task); + return Ok(false); + } + + if !task.has_work() { if file.has_older_records() { state.search_task = Some(task); return Ok(false); } - task.awaiting_older = false; - task.next_line = 0; - task.remaining = file.line_count(); + state.search_target = None; + state.search_match_ordinal = None; + state.search_match_target = None; + state.set_footer_message( + format!("not found: {}", task.query), + FooterMessageKind::Warning, + ); + return Ok(true); } let step = scan_search_chunk(file, &task)?; @@ -295,28 +539,13 @@ pub(in crate::viewer) fn process_search_step( return Ok(true); } - task.next_line = step.next_line; - let incomplete_index = !file.at_newer_boundary(); - task.remaining = task.remaining.saturating_sub(step.scanned); - if incomplete_index - && task.direction == SearchDirection::Forward - && task.remaining == 0 - && step.scanned > 0 - { - task.remaining = SEARCH_CHUNK_LINES; - } - let reached_unloaded_prefix = task.direction == SearchDirection::Forward - && file.has_older_records() - && task.next_line >= file.line_count(); - if reached_unloaded_prefix - || ((task.remaining == 0 || step.scanned == 0) && file.has_older_records()) - { - task.remaining = 0; + task.consume(step.scanned); + if !task.has_work() && file.has_older_records() { task.awaiting_older = true; state.search_task = Some(task); return Ok(false); } - if task.remaining == 0 || step.scanned == 0 { + if !task.has_work() || step.scanned == 0 { state.search_target = None; state.search_match_ordinal = None; state.search_match_target = None; @@ -387,7 +616,6 @@ fn floor_char_boundary(text: &str, mut index: usize) -> usize { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(in crate::viewer) struct SearchStep { pub(in crate::viewer) found: Option, - pub(in crate::viewer) next_line: usize, pub(in crate::viewer) scanned: usize, } @@ -405,52 +633,29 @@ pub(in crate::viewer) fn scan_search_forward( file: &dyn ViewFile, task: &SearchTask, ) -> Result { - let line_count = file.line_count(); - let exact_line_count = file.at_newer_boundary() && !file.has_older_records(); - if line_count == 0 || task.remaining == 0 { + let Some(span) = task.current_span() else { return Ok(SearchStep { found: None, - next_line: 0, scanned: 0, }); - } - - let mut next_line = task.next_line.min(line_count - 1); - let mut scanned = 0; - let limit = task.remaining.min(SEARCH_CHUNK_LINES); - - while scanned < limit { - let count = (line_count - next_line).min(limit - scanned); - let lines = file.read_window(next_line, count)?; - if lines.is_empty() { - break; - } - - for (offset, line) in lines.iter().enumerate() { - if let Some(byte_index) = line.find(&task.query) { - let found_line = next_line + offset; - return Ok(SearchStep { - found: Some(SearchTarget { - line: found_line, - byte_index, - }), - next_line: found_line, - scanned: scanned + offset + 1, - }); - } - } - - scanned += lines.len(); - next_line = next_line.saturating_add(lines.len()); - if next_line >= line_count { - next_line = if exact_line_count { 0 } else { line_count }; + }; + let count = span.len().min(SEARCH_CHUNK_LINES); + let lines = file.read_window(span.start, count)?; + for (offset, line) in lines.iter().enumerate() { + if let Some(byte_index) = line.find(&task.query) { + return Ok(SearchStep { + found: Some(SearchTarget { + line: span.start + offset, + byte_index, + }), + scanned: offset + 1, + }); } } Ok(SearchStep { found: None, - next_line, - scanned, + scanned: lines.len(), }) } @@ -458,48 +663,29 @@ pub(in crate::viewer) fn scan_search_backward( file: &dyn ViewFile, task: &SearchTask, ) -> Result { - let line_count = file.line_count(); - if line_count == 0 || task.remaining == 0 { + let Some(span) = task.current_span() else { return Ok(SearchStep { found: None, - next_line: 0, scanned: 0, }); - } - - let mut next_line = task.next_line.min(line_count - 1); - let mut scanned = 0; - let limit = task.remaining.min(SEARCH_CHUNK_LINES); - - while scanned < limit { - let count = (next_line + 1).min(limit - scanned); - let start = next_line + 1 - count; - let lines = file.read_window(start, count)?; - if lines.is_empty() { - break; - } - - for (offset, line) in lines.iter().enumerate().rev() { - if let Some(byte_index) = line.rfind(&task.query) { - let found_line = start + offset; - return Ok(SearchStep { - found: Some(SearchTarget { - line: found_line, - byte_index, - }), - next_line: found_line, - scanned: scanned + (count - offset), - }); - } + }; + let count = span.len().min(SEARCH_CHUNK_LINES); + let start = span.end.saturating_sub(count); + let lines = file.read_window(start, count)?; + for (offset, line) in lines.iter().enumerate().rev() { + if let Some(byte_index) = line.rfind(&task.query) { + return Ok(SearchStep { + found: Some(SearchTarget { + line: start + offset, + byte_index, + }), + scanned: lines.len().saturating_sub(offset), + }); } - - scanned += lines.len(); - next_line = start.checked_sub(1).unwrap_or(line_count - 1); } Ok(SearchStep { found: None, - next_line, - scanned, + scanned: lines.len(), }) } diff --git a/crates/fmtview-core/src/viewer/file/input/state.rs b/crates/fmtview-core/src/viewer/file/input/state.rs index f652bfd..f46a7be 100644 --- a/crates/fmtview-core/src/viewer/file/input/state.rs +++ b/crates/fmtview-core/src/viewer/file/input/state.rs @@ -235,10 +235,7 @@ impl ViewState { shift_target(&mut self.search_target, at, lines); shift_target(&mut self.structure_target, at, lines); if let Some(task) = self.search_task.as_mut() { - if !task.awaiting_older { - shift_index(&mut task.next_line, at, lines); - task.remaining = task.remaining.saturating_add(lines); - } + task.shift_for_insert(at, lines); } if let Some(index) = self.search_index.as_mut() { index.counted_lines = 0; @@ -262,6 +259,18 @@ impl ViewState { self.search_match_ordinal = None; self.clear_tool_navigation(); } + + pub(in crate::viewer) fn extend_for_append(&mut self, start: usize, end: usize) { + if start >= end { + return; + } + if let Some(task) = self.search_task.as_mut() { + task.extend_for_append(start, end); + } + if let Some(index) = self.search_index.as_mut() { + index.exact = false; + } + } } fn shift_index(value: &mut usize, at: usize, lines: usize) { diff --git a/crates/fmtview-core/src/viewer/tests/search.rs b/crates/fmtview-core/src/viewer/tests/search.rs index 1e3ebcd..a71bc68 100644 --- a/crates/fmtview-core/src/viewer/tests/search.rs +++ b/crates/fmtview-core/src/viewer/tests/search.rs @@ -728,3 +728,151 @@ fn search_highlight_ignores_chat_role_gutter() { assert_eq!(background_cell_count(&[highlighted]), 0); } + +struct MutableSearchFile { + lines: std::cell::RefCell>, + reads: std::cell::RefCell>, +} + +impl MutableSearchFile { + fn with_len(line_count: usize) -> Self { + Self { + lines: std::cell::RefCell::new( + (0..line_count).map(|line| format!("line-{line}")).collect(), + ), + reads: std::cell::RefCell::new(Vec::new()), + } + } + + fn append(&self, lines: impl IntoIterator) -> (usize, usize) { + let mut current = self.lines.borrow_mut(); + let start = current.len(); + current.extend(lines); + (start, current.len()) + } + + fn read_counts(&self) -> Vec { + let line_count = self.line_count(); + let mut counts = vec![0_usize; line_count]; + for line in self.reads.borrow().iter().copied() { + counts[line] = counts[line].saturating_add(1); + } + counts + } +} + +impl ViewFile for MutableSearchFile { + fn label(&self) -> &str { + "mutable-search" + } + + fn line_count(&self) -> usize { + self.lines.borrow().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> { + let lines = self.lines.borrow(); + let end = start.saturating_add(count).min(lines.len()); + self.reads.borrow_mut().extend(start..end); + Ok(lines[start..end].to_vec()) + } +} + +#[test] +fn forward_search_does_not_rescan_wrapped_history_before_multi_append_delta() { + let original_len = crate::viewer::file::SEARCH_CHUNK_LINES + 2_000; + let file = MutableSearchFile::with_len(original_len); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-forward-append".to_owned(), + SearchDirection::Forward, + original_len - 5, + original_len, + ); + + assert!(!process_search_step(&file, &mut state).unwrap()); + assert!(!process_search_step(&file, &mut state).unwrap()); + let (first_start, first_end) = file.append(["append-without-match".to_owned()]); + state.extend_for_append(first_start, first_end); + let (second_start, second_end) = file.append([ + "needle-forward-append".to_owned(), + "append-after-match".to_owned(), + ]); + state.extend_for_append(second_start, second_end); + assert_eq!(state.search_task.as_ref().unwrap().span_count(), 2); + + while state.search_task.is_some() { + process_search_step(&file, &mut state).unwrap(); + } + + assert_eq!(state.search_cursor, Some(original_len + 1)); + let counts = file.read_counts(); + assert!(counts[..original_len].iter().all(|count| *count <= 1)); + assert_eq!(counts[original_len], 1); + assert_eq!(counts[original_len + 1], 1); + assert_eq!(counts[original_len + 2], 1); +} + +#[test] +fn backward_search_does_not_rescan_wrapped_history_before_append_delta() { + let original_len = crate::viewer::file::SEARCH_CHUNK_LINES + 2_000; + let file = MutableSearchFile::with_len(original_len); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-backward-append".to_owned(), + SearchDirection::Backward, + 4, + original_len, + ); + + assert!(!process_search_step(&file, &mut state).unwrap()); + assert!(!process_search_step(&file, &mut state).unwrap()); + let (start, end) = file.append([ + "append-before-match".to_owned(), + "needle-backward-append".to_owned(), + ]); + state.extend_for_append(start, end); + + while state.search_task.is_some() { + process_search_step(&file, &mut state).unwrap(); + } + + assert_eq!(state.search_cursor, Some(original_len + 1)); + let counts = file.read_counts(); + assert!(counts[..original_len].iter().all(|count| *count <= 1)); + assert_eq!(counts[original_len], 1); + assert_eq!(counts[original_len + 1], 1); +} + +#[test] +fn append_invalidates_and_recounts_an_exact_search_index() { + let file = MutableSearchFile::with_len(3); + let mut state = ViewState::default(); + start_search( + &mut state, + "needle-index-append".to_owned(), + SearchDirection::Forward, + 0, + file.line_count(), + ); + assert!(process_search_index_step(&file, &mut state).unwrap()); + assert!(state.search_index.as_ref().unwrap().exact); + assert_eq!(state.search_index.as_ref().unwrap().matches, 0); + + let (start, end) = file.append(["needle-index-append".to_owned()]); + state.extend_for_append(start, end); + assert!(!state.search_index.as_ref().unwrap().exact); + assert!(process_search_index_step(&file, &mut state).unwrap()); + assert!(state.search_index.as_ref().unwrap().exact); + assert_eq!(state.search_index.as_ref().unwrap().matches, 1); +} diff --git a/crates/fmtview-core/tests/record_timeline.rs b/crates/fmtview-core/tests/record_timeline.rs index e54f2b9..0390819 100644 --- a/crates/fmtview-core/tests/record_timeline.rs +++ b/crates/fmtview-core/tests/record_timeline.rs @@ -965,6 +965,180 @@ fn repeated_forward_search_wraps_into_lazily_loaded_older_records() { assert!(wrapped.contains("needle-old"), "{wrapped}"); } +#[test] +fn wrapped_forward_search_waits_for_the_true_prefix_before_choosing_a_match() { + let (_handle, timeline) = fake_timeline((0..200).map(|index| match index { + 0 => b"{\"message\":\"needle-oldest\"}\n".to_vec(), + 100 => b"{\"message\":\"needle-newer-prefix-batch\"}\n".to_vec(), + 199 => b"{\"message\":\"needle-tail\"}\n".to_vec(), + _ => record(index), + })); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(2, 4096), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 10); + viewer.render(size, None).unwrap(); + + enter_search(&mut viewer, size, "needle"); + advance_until_idle(&mut viewer); + assert!(frame_text(viewer.render(size, None).unwrap()).contains("needle-tail")); + + viewer.handle_event(key(KeyCode::Char('n')), FileViewer::page_for_size(size)); + advance_until_idle(&mut viewer); + let wrapped = frame_text(viewer.render(size, None).unwrap()); + + assert!(wrapped.contains("needle-oldest"), "{wrapped}"); + assert!(!wrapped.contains("needle-newer-prefix-batch"), "{wrapped}"); +} + +#[test] +fn active_forward_search_includes_newer_records_arriving_at_the_boundary() { + let (handle, timeline) = fake_timeline([record(0), record(1)]); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(16, 4096), + ) + .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)); + viewer.render(size, None).unwrap(); + enter_search(&mut viewer, size, "needle-after-search-started"); + + handle.append(b"{\"message\":\"needle-after-search-started\"}\n".to_vec()); + viewer.preload().unwrap(); + advance_until_idle(&mut viewer); + let frame = viewer.render(size, None).unwrap(); + + assert!(frame_text(frame).contains("needle-after-search-started")); +} + +#[test] +fn append_match_is_found_before_lazy_older_history_finishes_loading() { + let (handle, timeline) = fake_timeline((0..10_000).map(record)); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(2, 4096), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 8); + viewer.render(size, None).unwrap(); + enter_search(&mut viewer, size, "needle-while-awaiting-older"); + + viewer.advance(Instant::now()).unwrap(); + handle.append(b"{\"message\":\"needle-while-awaiting-older\"}\n".to_vec()); + viewer.preload().unwrap(); + advance_until_idle(&mut viewer); + let frame = viewer.render(size, None).unwrap(); + let footer = frame.footer_text.clone(); + let text = frame_text(frame); + + assert!(text.contains("needle-while-awaiting-older"), "{footer}"); + assert!( + handle.older_calls() < 10, + "append search loaded {} older batches before finding the live match", + handle.older_calls() + ); +} + +#[test] +fn active_search_crosses_a_reset_tail_and_lazily_inserted_older_records() { + let (handle, timeline) = fake_timeline((0..20).map(record)); + let file = RecordTimelineViewFile::with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(4, 4096), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 8); + viewer.render(size, None).unwrap(); + enter_search(&mut viewer, size, "needle-in-reset-prefix"); + + handle.replace((0..140).map(|index| { + if index == 0 { + b"{\"message\":\"needle-in-reset-prefix\"}\n".to_vec() + } else { + format!("{{\"message\":\"replacement-{index}\"}}\n").into_bytes() + } + })); + viewer.preload().unwrap(); + advance_until_idle(&mut viewer); + let frame = viewer.render(size, None).unwrap(); + let footer = frame.footer_text.clone(); + let text = frame_text(frame); + + assert!(text.contains("needle-in-reset-prefix"), "{footer}"); +} + +#[test] +fn repeated_search_finds_appended_match_after_complete_miss_in_each_follow_state() { + for (mode, expected_follow) in [ + ("following", "follow:on"), + ("detached", "follow:detached"), + ("paused", "follow:off"), + ] { + 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(); + match mode { + "detached" => { + viewer.handle_event(key(KeyCode::Home), FileViewer::page_for_size(size)); + } + "paused" => { + viewer.handle_event(key(KeyCode::Char('f')), FileViewer::page_for_size(size)); + } + "following" => {} + _ => unreachable!(), + } + viewer.render(size, None).unwrap(); + + enter_search(&mut viewer, size, "needle-after-complete-miss"); + advance_until_idle(&mut viewer); + let missed = viewer.render(size, None).unwrap(); + assert!( + missed + .footer_text + .contains("not found: needle-after-complete-miss"), + "mode={mode} footer={}", + missed.footer_text + ); + + handle.append(b"{\"message\":\"needle-after-complete-miss\"}\n".to_vec()); + viewer.preload().unwrap(); + viewer.handle_event(key(KeyCode::Char('n')), FileViewer::page_for_size(size)); + advance_until_idle(&mut viewer); + let appended = viewer.render(size, None).unwrap(); + let footer = appended.footer_text.clone(); + let text = frame_text(appended); + + assert!( + text.contains("needle-after-complete-miss"), + "mode={mode} footer={footer}" + ); + assert!(footer.contains("1/1"), "mode={mode} footer={footer}"); + assert!( + footer.contains(expected_follow), + "mode={mode} footer={footer}" + ); + } +} + #[test] fn file_timeline_preserves_crlf_empty_records_and_ignores_incomplete_eof() { let mut temp = NamedTempFile::new().unwrap(); diff --git a/docs/architecture.md b/docs/architecture.md index 31f4c9a..99b09cd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -330,10 +330,15 @@ The contract has these invariants: fixed epoch boundary and cannot interleave ahead of stale old-epoch history. `FileRecordTimeline` implements the seam for growing newline-delimited files. -It locates committed EOF from bounded reverse chunks, never exposes an +It locates committed EOF with fixed-size reverse-read buffers, never exposes an incomplete final line, and lets bounded forward loads do the record work after -a refresh. Refresh validates bounded start/middle/end samples of committed -history independently of file timestamps, catching same-identity rewrites when +a refresh. Committed-boundary discovery is proportional to the incomplete EOF +suffix, so a newline-free file can require reading back to offset zero even +though the scanner's memory stays bounded. Loading the initial committed tail is +a separate operation capped at 128 records and 4 MiB, except that one record may +exceed the byte budget. Refresh validates bounded start/middle/end samples of +committed history independently of file timestamps, catching same-identity +rewrites when one of those windows changes without indexing the whole file. Inode/device identity is used on Unix; portable fallbacks never treat file length as identity. diff --git a/docs/performance.md b/docs/performance.md index ad6a6c9..2f2095c 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -142,8 +142,12 @@ Load metrics: `record-stream/huge-media`. - `timeline tail-first open+format` measures reverse tail discovery, formatting the initial bounded record batch, spooling/indexing it, and reading the last - viewport. Fixture generation is outside the timed region, and correctness - tests separately assert bounded source bytes with instrumentation. + viewport. Fixture generation is outside the timed region. Correctness tests + separately assert bounded work for a normally delimited million-record file; + a newline-free EOF suffix remains proportional to that single incomplete + record because no source can prove the absence of an earlier delimiter + without reading it. The subsequent initial committed-tail load is capped at + 128 records and 4 MiB, except that one record may exceed the byte budget. - `timeline older prepend+format` measures one bounded backward record load, formatting/spooling, and insertion into the session indexes. - `timeline append refresh+format` measures reverse committed-boundary refresh, From 408f9e57b31c033be7a46f3ec5dd0d71a63389d6 Mon Sep 17 00:00:00 2001 From: "Xinyao (Morry) Niu" Date: Wed, 22 Jul 2026 18:55:57 +0800 Subject: [PATCH 2/3] test(search): cover multi-batch prefix ordering Grow the wrapped forward-search fixture beyond one lazy older-load batch so the regression proves a newer reverse-loaded match cannot outrank the true source prefix. --- crates/fmtview-core/tests/record_timeline.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fmtview-core/tests/record_timeline.rs b/crates/fmtview-core/tests/record_timeline.rs index 0390819..de9c256 100644 --- a/crates/fmtview-core/tests/record_timeline.rs +++ b/crates/fmtview-core/tests/record_timeline.rs @@ -967,10 +967,10 @@ fn repeated_forward_search_wraps_into_lazily_loaded_older_records() { #[test] fn wrapped_forward_search_waits_for_the_true_prefix_before_choosing_a_match() { - let (_handle, timeline) = fake_timeline((0..200).map(|index| match index { + let (_handle, timeline) = fake_timeline((0..1_200).map(|index| match index { 0 => b"{\"message\":\"needle-oldest\"}\n".to_vec(), - 100 => b"{\"message\":\"needle-newer-prefix-batch\"}\n".to_vec(), - 199 => b"{\"message\":\"needle-tail\"}\n".to_vec(), + 700 => b"{\"message\":\"needle-newer-prefix-batch\"}\n".to_vec(), + 1_199 => b"{\"message\":\"needle-tail\"}\n".to_vec(), _ => record(index), })); let file = RecordTimelineViewFile::with_initial_limit( From cd8e03668cf0a1e2aa91b012c05dbd1ecebd1b39 Mon Sep 17 00:00:00 2001 From: "Xinyao (Morry) Niu" Date: Wed, 22 Jul 2026 19:05:37 +0800 Subject: [PATCH 3/3] 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 [