From 125e6eb443e88bc79533631e676d03a4865688ab Mon Sep 17 00:00:00 2001 From: "Xinyao (Morry) Niu" Date: Wed, 22 Jul 2026 18:18:13 +0800 Subject: [PATCH] fix(view): keep snapshots anchored at the lazy tail Separate initial tail anchoring from follow state and arm bounded older preload only after backward navigation intent. Remove the misleading public notice option, which rendered ordinary embedder text as an error. --- README.md | 1 - crates/fmtview-core/src/load/timeline.rs | 4 ++ crates/fmtview-core/src/load/view_file.rs | 4 ++ crates/fmtview-core/src/viewer/file.rs | 24 ++++++- .../src/viewer/file/input/state.rs | 2 + crates/fmtview-core/tests/record_timeline.rs | 68 ++++++++++++++++++- examples/embed-timeline.rs | 4 +- src/view.rs | 27 +++++--- 8 files changed, 116 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index f633689..41c4d74 100644 --- a/README.md +++ b/README.md @@ -300,7 +300,6 @@ use fmtview::view::{self, RecordTimeline, Result, ViewOptions}; fn inspect(source: Box, follow: bool) -> Result<()> { let mut options = ViewOptions::default(); options.follow = follow; - options.notice = Some("opened by my application".to_owned()); view::run(source, options) } ``` diff --git a/crates/fmtview-core/src/load/timeline.rs b/crates/fmtview-core/src/load/timeline.rs index c90afc8..d7dfde5 100644 --- a/crates/fmtview-core/src/load/timeline.rs +++ b/crates/fmtview-core/src/load/timeline.rs @@ -145,6 +145,10 @@ impl ViewFile for RecordTimelineViewFile { self.follow } + fn starts_at_tail(&self) -> bool { + true + } + fn has_older_records(&self) -> bool { !self.state.borrow().older_end } diff --git a/crates/fmtview-core/src/load/view_file.rs b/crates/fmtview-core/src/load/view_file.rs index 4e1a0c4..273f508 100644 --- a/crates/fmtview-core/src/load/view_file.rs +++ b/crates/fmtview-core/src/load/view_file.rs @@ -31,6 +31,10 @@ pub trait ViewFile { fn is_follow_source(&self) -> bool { false } + /// Whether the initially loaded window is anchored at the newer boundary. + fn starts_at_tail(&self) -> bool { + false + } fn has_older_records(&self) -> bool { false } diff --git a/crates/fmtview-core/src/viewer/file.rs b/crates/fmtview-core/src/viewer/file.rs index 3c8ab77..f1b06cb 100644 --- a/crates/fmtview-core/src/viewer/file.rs +++ b/crates/fmtview-core/src/viewer/file.rs @@ -80,11 +80,15 @@ struct RawRecordOverlay { impl FileViewer { pub fn new(file: Box, mode: FormatKind, notice: Option) -> Self { let mut state = ViewState::default(); - if file.is_follow_source() { - state.follow = Some(FollowState::Following); + if file.starts_at_tail() { state.viewport_at_tail = true; + state.preserve_tail_on_next_draw = true; + state.older_preload_armed = false; set_file_end(&mut state, file.line_count()); } + if file.is_follow_source() { + state.follow = Some(FollowState::Following); + } state.raw_record_available = file.supports_raw_records(); if let Some(message) = notice { state.set_notice(message, Instant::now(), NOTICE_DURATION); @@ -198,7 +202,10 @@ impl FileViewer { LAZY_PRELOAD_BUDGET, )? }; - if self.state.top <= TIMELINE_OLDER_THRESHOLD_LINES && self.file.has_older_records() { + if self.state.older_preload_armed + && self.state.top <= TIMELINE_OLDER_THRESHOLD_LINES + && self.file.has_older_records() + { let older = self .file .load_older_records(LAZY_PRELOAD_RECORDS, TIMELINE_PRELOAD_BYTES)?; @@ -289,6 +296,15 @@ impl FileViewer { self.file.has_older_records(), page, ); + if self.file.starts_at_tail() && !had_active_prompt { + if event_moves_away_from_tail(event, self.state.wrap) + && (action.dirty || self.file.has_older_records()) + { + self.state.older_preload_armed = true; + } else if event_forces_tail(event) { + self.state.older_preload_armed = false; + } + } if self.file.is_follow_source() && !had_active_prompt { if action.dirty && event_moves_away_from_tail(event, self.state.wrap) { if self.state.follow == Some(FollowState::Following) { @@ -316,6 +332,7 @@ impl FileViewer { if !action.dirty || self.state.top >= self.file.line_count().saturating_sub(1) { self.state.follow = Some(FollowState::Following); self.state.viewport_at_tail = true; + self.state.older_preload_armed = false; self.state.follow_reattach_pending = false; set_file_end(&mut self.state, self.file.line_count()); action.dirty = true; @@ -416,6 +433,7 @@ impl FileViewer { fn enable_follow_tail(&mut self) -> ViewerAction { self.state.follow = Some(FollowState::Following); self.state.viewport_at_tail = true; + self.state.older_preload_armed = false; self.state.follow_reattach_pending = false; set_file_end(&mut self.state, self.file.line_count()); ViewerAction { diff --git a/crates/fmtview-core/src/viewer/file/input/state.rs b/crates/fmtview-core/src/viewer/file/input/state.rs index 408163b..f652bfd 100644 --- a/crates/fmtview-core/src/viewer/file/input/state.rs +++ b/crates/fmtview-core/src/viewer/file/input/state.rs @@ -53,6 +53,7 @@ pub(in crate::viewer) struct ViewState { pub(in crate::viewer) tool_target: Option, pub(in crate::viewer) viewport_at_tail: bool, pub(in crate::viewer) preserve_tail_on_next_draw: bool, + pub(in crate::viewer) older_preload_armed: bool, pub(in crate::viewer) follow: Option, pub(in crate::viewer) follow_reattach_pending: bool, pub(in crate::viewer) mouse_capture: bool, @@ -90,6 +91,7 @@ impl Default for ViewState { tool_target: None, viewport_at_tail: false, preserve_tail_on_next_draw: false, + older_preload_armed: true, follow: None, follow_reattach_pending: false, mouse_capture: true, diff --git a/crates/fmtview-core/tests/record_timeline.rs b/crates/fmtview-core/tests/record_timeline.rs index 00c12c5..e54f2b9 100644 --- a/crates/fmtview-core/tests/record_timeline.rs +++ b/crates/fmtview-core/tests/record_timeline.rs @@ -65,9 +65,11 @@ fn snapshot_timeline_lazily_loads_older_without_refresh_or_follow_controls() { ); handle.append(record(130)); - assert!(viewer.preload().unwrap()); + assert!(!viewer.preload().unwrap()); assert_eq!(handle.refresh_calls(), 0); viewer.handle_event(key(KeyCode::Char('g')), FileViewer::page_for_size(size)); + assert!(viewer.preload().unwrap()); + viewer.handle_event(key(KeyCode::Char('g')), FileViewer::page_for_size(size)); let first_older_batch = frame_text(viewer.render(size, None).unwrap()); assert!( first_older_batch.contains("record-64"), @@ -86,6 +88,62 @@ fn snapshot_timeline_lazily_loads_older_without_refresh_or_follow_controls() { assert_eq!(handle.refresh_calls(), 0); } +#[test] +fn snapshot_opens_at_tail_without_eagerly_loading_older_records() { + let (handle, timeline) = fake_timeline((0..130).map(record)); + let file = RecordTimelineViewFile::snapshot_with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(8, 4096), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 8); + + let tail = viewer.render(size, None).unwrap(); + let tail_text = frame_text(tail); + assert!(tail_text.contains("record-129"), "{tail_text}"); + assert!(!tail_text.contains("record-122"), "{tail_text}"); + let repeated_tail = frame_text(viewer.render(size, None).unwrap()); + assert!(repeated_tail.contains("record-129"), "{repeated_tail}"); + assert!(!viewer.preload().unwrap()); + assert_eq!(handle.older_calls(), 1); + assert_eq!(handle.refresh_calls(), 0); + + viewer.handle_event(key(KeyCode::PageUp), FileViewer::page_for_size(size)); + assert!(viewer.preload().unwrap()); + assert_eq!(handle.older_calls(), 2); + + viewer.handle_event(key(KeyCode::Char('G')), FileViewer::page_for_size(size)); + let returned_tail = frame_text(viewer.render(size, None).unwrap()); + assert!(returned_tail.contains("record-129"), "{returned_tail}"); + assert!(!viewer.preload().unwrap()); + assert_eq!(handle.older_calls(), 2); +} + +#[test] +fn snapshot_backward_intent_loads_older_when_the_tail_batch_already_fits() { + let (handle, timeline) = fake_timeline((0..10).map(record)); + let file = RecordTimelineViewFile::snapshot_with_initial_limit( + Box::new(timeline), + JSONL, + RecordLoadLimit::new(1, 4096), + ) + .unwrap(); + let mut viewer = FileViewer::new(Box::new(file), FormatKind::Jsonl, None); + let size = Size::new(60, 12); + + let tail = frame_text(viewer.render(size, None).unwrap()); + assert!(tail.contains("record-9"), "{tail}"); + assert!(!viewer.preload().unwrap()); + assert_eq!(handle.older_calls(), 1); + + let action = viewer.handle_event(key(KeyCode::PageUp), FileViewer::page_for_size(size)); + assert!(!action.dirty); + assert!(viewer.preload().unwrap()); + assert_eq!(handle.older_calls(), 2); +} + #[test] fn snapshot_search_and_structure_load_older_without_refreshing() { let (search_handle, search_timeline) = fake_timeline((0..80).map(record)); @@ -112,6 +170,7 @@ fn snapshot_search_and_structure_load_older_without_refreshing() { ) .unwrap(); let mut structure_viewer = FileViewer::new(Box::new(structure_file), FormatKind::Jsonl, None); + structure_viewer.render(size, None).unwrap(); structure_viewer.handle_event(key(KeyCode::Char('[')), FileViewer::page_for_size(size)); advance_until_idle(&mut structure_viewer); let structure_frame = frame_text(structure_viewer.render(size, None).unwrap()); @@ -156,6 +215,7 @@ fn exhausted_snapshot_reports_no_previous_structure_without_rearming() { 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::Char('[')), FileViewer::page_for_size(size)); assert!(!viewer.needs_immediate_advance()); assert!(!viewer.advance(Instant::now()).unwrap()); @@ -597,15 +657,17 @@ fn prepending_older_records_preserves_the_viewport_anchor() { .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::PageUp), FileViewer::page_for_size(size)); let before = viewer.render(size, None).unwrap(); let before_top = before.position.top; let before_text = frame_text(before); - assert!(before_text.contains("record-99"), "{before_text:?}"); + assert!(before_text.contains("record-98"), "{before_text:?}"); assert!(viewer.preload().unwrap()); let after = viewer.render(size, None).unwrap(); assert!(after.position.top > before_top); - assert!(frame_text(after).contains("record-99")); + assert!(frame_text(after).contains("record-98")); } #[test] diff --git a/examples/embed-timeline.rs b/examples/embed-timeline.rs index 01f070e..aeb7c73 100644 --- a/examples/embed-timeline.rs +++ b/examples/embed-timeline.rs @@ -137,7 +137,5 @@ fn main() -> Result<()> { b"{\"ref\":\"m2\",\"role\":\"assistant\",\"content\":[{\"type\":\"tool_call\",\"id\":\"call_1\",\"name\":\"bash\",\"arguments\":\"{\\\"cmd\\\":\\\"cargo test\\\"}\"}]}\n".to_vec(), b"{\"ref\":\"m3\",\"role\":\"tool\",\"content\":[{\"type\":\"tool_result\",\"call_id\":\"call_1\",\"content\":\"ok\"}]}\n".to_vec(), ]); - let mut options = ViewOptions::default(); - options.notice = Some("embedded through fmtview::view".to_owned()); - view::run(Box::new(timeline), options) + view::run(Box::new(timeline), ViewOptions::default()) } diff --git a/src/view.rs b/src/view.rs index f21fe92..4cc1fba 100644 --- a/src/view.rs +++ b/src/view.rs @@ -69,8 +69,6 @@ pub type Result = anyhow::Result; pub struct ViewOptions { /// Number of spaces used when formatting each JSONL record. pub indent: usize, - /// Optional transient message shown when the viewer opens. - pub notice: Option, /// Refresh newer records and enable attached/detached/paused follow state. pub follow: bool, } @@ -79,7 +77,6 @@ impl Default for ViewOptions { fn default() -> Self { Self { indent: 2, - notice: None, follow: false, } } @@ -93,13 +90,17 @@ impl Default for ViewOptions { /// crossterm event loop, and terminal cleanup. pub fn run(source: Box, options: ViewOptions) -> Result<()> { validate_options(&options)?; - if !io::stdout().is_terminal() { - bail!("embedded fmtview requires an interactive terminal on stdout"); - } + require_interactive_stdout(io::stdout().is_terminal())?; - let notice = options.notice.clone(); let file = open_timeline(source, &options)?; - crate::viewer::run(file, FormatKind::Jsonl, notice) + crate::viewer::run(file, FormatKind::Jsonl, None) +} + +fn require_interactive_stdout(is_terminal: bool) -> Result<()> { + if !is_terminal { + bail!("embedded fmtview requires an interactive terminal on stdout"); + } + Ok(()) } fn validate_options(options: &ViewOptions) -> Result<()> { @@ -192,4 +193,14 @@ mod tests { assert!(validate_options(&options).is_err()); } + + #[test] + fn rejects_redirected_stdout_before_entering_the_terminal() { + let error = require_interactive_stdout(false).unwrap_err(); + + assert_eq!( + error.to_string(), + "embedded fmtview requires an interactive terminal on stdout" + ); + } }