From 0e1103efd2d739014df6b282138a1f37dcc4f26c Mon Sep 17 00:00:00 2001 From: Hal Frigaard <4559349+HalFrgrd@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:13:07 +0100 Subject: [PATCH 1/3] clean up logic and log --- crates/flycontent/src/palette.rs | 4 +- src/history/backend.rs | 83 +++++++++++++++++++++++++++++--- src/history/importing.rs | 37 ++++++-------- 3 files changed, 96 insertions(+), 28 deletions(-) diff --git a/crates/flycontent/src/palette.rs b/crates/flycontent/src/palette.rs index 5ecc2ce2..bb166817 100644 --- a/crates/flycontent/src/palette.rs +++ b/crates/flycontent/src/palette.rs @@ -419,7 +419,9 @@ impl Palette { .fg(Color::Yellow) .add_modifier(Modifier::BOLD), right_click_menu: Style::default().fg(Color::Black).bg(Color::Gray), - scrollbar: Style::default().fg(Color::White).bg(Color::Rgb(100, 100, 100)), + scrollbar: Style::default() + .fg(Color::White) + .bg(Color::Rgb(100, 100, 100)), rainbow_brackets: [ Style::default().fg(Color::Rgb(255, 215, 0)), // gold Style::default().fg(Color::Rgb(255, 100, 100)), // coral diff --git a/src/history/backend.rs b/src/history/backend.rs index f149cdf8..ce9e39a7 100644 --- a/src/history/backend.rs +++ b/src/history/backend.rs @@ -206,7 +206,7 @@ pub(super) struct JsonlFetchResult { pub(super) last_read_offset: Option, } -fn fetch_flyline_jsonl_history_from_offset( +pub(super) fn fetch_flyline_jsonl_history_from_offset( path: &Path, last_offset: Option<&LastJsonlReadOffset>, ) -> anyhow::Result { @@ -301,12 +301,36 @@ fn fetch_flyline_jsonl_history_from_offset( let mut events = Vec::new(); let mut line_start_pos = actual_offset; let mut last_seen_id = last_seen_event_id.map(String::from); + let mut unparseable_count = 0usize; + let mut buf = String::new(); - while let Some((event, bytes_read)) = read_event_from_reader(&mut reader) { - last_seen_id = Some(event.id().to_string()); - last_seen_start_offset = Some(line_start_pos); - line_start_pos += bytes_read; - events.push(event); + while let Ok(bytes_read) = reader.read_line(&mut buf) { + if bytes_read == 0 { + break; + } + let trimmed = buf.trim(); + if !trimmed.is_empty() { + match serde_json::from_str::(trimmed) { + Ok(event) => { + last_seen_id = Some(event.id().to_string()); + last_seen_start_offset = Some(line_start_pos); + events.push(event); + } + Err(_) => { + unparseable_count += 1; + } + } + } + line_start_pos += bytes_read as u64; + buf.clear(); + } + + if unparseable_count > 0 { + log::warn!( + "Failed to parse {} lines from Flyline JSONL history file {:?}", + unparseable_count, + path + ); } let result_last_offset = match (last_seen_start_offset, last_seen_id) { @@ -683,4 +707,51 @@ mod tests { let _ = std::fs::remove_file(&temp_file); } + + #[test] + fn test_fetch_jsonl_unparseable_lines_counted() { + use std::io::Write; + let temp_file = std::env::temp_dir().join(format!( + "flyline_test_unparseable_{}.jsonl", + uuid::Uuid::now_v7() + )); + let _ = std::fs::remove_file(&temp_file); + + let event1 = HistoryJsonlEvent::Start { + id: "cmd-1".to_string(), + timestamp: TimestampNanos::new(100), + command: "echo valid".to_string(), + cwd: None, + hostname: None, + session: "sess".to_string(), + }; + append_jsonl_history_event(&event1, &temp_file).unwrap(); + + // Write corrupt/unparseable lines + { + let mut file = std::fs::OpenOptions::new() + .append(true) + .open(&temp_file) + .unwrap(); + writeln!(file, "not valid json").unwrap(); + writeln!(file, "{{corrupt json").unwrap(); + } + + let event2 = HistoryJsonlEvent::Start { + id: "cmd-2".to_string(), + timestamp: TimestampNanos::new(200), + command: "echo valid 2".to_string(), + cwd: None, + hostname: None, + session: "sess".to_string(), + }; + append_jsonl_history_event(&event2, &temp_file).unwrap(); + + let res = fetch_jsonl_new_entries_from_offset(&temp_file, None).unwrap(); + assert_eq!(res.new_entries.len(), 2); + assert_eq!(res.new_entries[0].command, "echo valid"); + assert_eq!(res.new_entries[1].command, "echo valid 2"); + + let _ = std::fs::remove_file(&temp_file); + } } diff --git a/src/history/importing.rs b/src/history/importing.rs index 6228aa76..f2c0ce6f 100644 --- a/src/history/importing.rs +++ b/src/history/importing.rs @@ -4,7 +4,9 @@ use std::io::{BufRead, BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::Command; -use super::backend::{HistoryJsonlEvent, append_jsonl_history_events, is_file_empty_or_missing}; +use super::backend::{ + HistoryJsonlEvent, append_jsonl_history_events, fetch_flyline_jsonl_history_from_offset, +}; use super::{HistoryEntry, HistoryManager, TimestampNanos}; fn is_sqlite_db_file(path: &Path) -> bool { @@ -18,26 +20,19 @@ fn is_sqlite_db_file(path: &Path) -> bool { } fn load_existing_jsonl_dedup_set(target_jsonl_path: &Path) -> HashSet<(u64, String)> { - let mut seen_set = HashSet::new(); - if !is_file_empty_or_missing(target_jsonl_path) { - if let Ok(file) = File::open(target_jsonl_path) { - let reader = BufReader::new(file); - for line in reader.lines().map_while(Result::ok) { - let trimmed = line.trim(); - if !trimmed.is_empty() { - if let Ok(event) = serde_json::from_str::(trimmed) { - if let HistoryJsonlEvent::Start { - timestamp, command, .. - } = event - { - seen_set.insert((timestamp.as_seconds(), command)); - } - } - } - } - } - } - seen_set + fetch_flyline_jsonl_history_from_offset(target_jsonl_path, None) + .map(|res| { + res.events + .into_iter() + .filter_map(|event| match event { + HistoryJsonlEvent::Start { + timestamp, command, .. + } => Some((timestamp.as_seconds(), command)), + HistoryJsonlEvent::End { .. } => None, + }) + .collect() + }) + .unwrap_or_default() } fn append_imported_entry_to_jsonl( From 9c5c804248fdb77e2765af8e2d00b82872da9a0d Mon Sep 17 00:00:00 2001 From: Hal Frigaard <4559349+HalFrgrd@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:22:20 +0100 Subject: [PATCH 2/3] clean up --- src/history/mod.rs | 43 +++++-------------------------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/src/history/mod.rs b/src/history/mod.rs index 0a03e53c..9e2a8ba3 100644 --- a/src/history/mod.rs +++ b/src/history/mod.rs @@ -535,15 +535,6 @@ impl HistoryManager { normalized } - fn merge_history_entries( - zsh_entries: Vec, - bash_entries: Vec, - ) -> Vec { - let mut all = zsh_entries; - all.extend(bash_entries); - Self::normalize_entries(all) - } - #[cfg(test)] pub fn parse_bash_history_from_memory() -> Vec { Vec::new() @@ -646,15 +637,13 @@ impl HistoryManager { self.last_search_prefix = None; self.last_buffered_command = None; self.last_word_insert_index = None; - let bash_entries = shell::backend().parse_history_from_memory(); - Self::log_recent_entries(&bash_entries, "bash"); - let entries = if let Some(zsh_path) = zsh_history_path { + let mut entries = shell::backend().parse_history_from_memory(); + Self::log_recent_entries(&entries, "bash"); + if let Some(zsh_path) = zsh_history_path { let zsh_entries = Self::parse_zsh_history(Some(zsh_path)); Self::log_recent_entries(&zsh_entries, "Zsh"); - Self::merge_history_entries(zsh_entries, bash_entries) - } else { - bash_entries - }; + entries.extend(zsh_entries); + } self.entries = Self::normalize_entries(entries); self.index = self.entries.len(); self.fuzzy_search.clear_cache(); @@ -1626,28 +1615,6 @@ git status assert_eq!(normalized[1].index, 1); } - #[test] - fn test_merge_history_entries_dedups_adjacent_and_reindexes() { - let zsh_entries = vec![ - HistoryEntry::new(Some(1), 10, "echo hi".to_string()), - HistoryEntry::new(Some(3), 11, "pwd".to_string()), - ]; - let bash_entries = vec![ - HistoryEntry::new(Some(1), 20, "echo hi".to_string()), - HistoryEntry::new(Some(4), 21, "ls".to_string()), - ]; - - let merged = HistoryManager::merge_history_entries(zsh_entries, bash_entries); - - assert_eq!(merged.len(), 3); - assert_eq!(merged[0].command, "echo hi"); - assert_eq!(merged[0].index, 0); - assert_eq!(merged[1].command, "pwd"); - assert_eq!(merged[1].index, 1); - assert_eq!(merged[2].command, "ls"); - assert_eq!(merged[2].index, 2); - } - #[test] fn test_last_word_insert_logic() { let mut hm = HistoryManager::default(); From 9ad3b78b508da885ce7c9fb3d8a3ba19c5d9621d Mon Sep 17 00:00:00 2001 From: Hal Frigaard <4559349+HalFrgrd@users.noreply.github.com> Date: Tue, 18 Aug 2026 22:29:13 +0100 Subject: [PATCH 3/3] Fix clippy warnings and document CI checks in AGENTS.md --- AGENTS.md | 7 ++++++- src/active_suggestions.rs | 14 +++++++------- src/agent_mode.rs | 6 ++---- src/app/actions/keyboard.rs | 4 ++-- src/completions/tab_completion.rs | 2 +- src/history/importing.rs | 2 +- src/perf.rs | 2 +- 7 files changed, 20 insertions(+), 17 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2d8431ed..9c5d94d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,10 +30,15 @@ cargo test -p flycomp # Format the codebase after making changes cargo fmt + +# Run Code Quality CI checks locally +cargo fmt --all -- --check +cargo clippy --workspace --all-features -- -D warnings +cargo doc --workspace --no-deps --document-private-items ``` > [!TIP] -> Avoid running the full `cargo test` suite locally. The integration tests (`tests/docker_integration_tests.rs`) spawn Docker containers testing multiple versions of Bash, which is extremely slow. Prefer running `cargo test --lib` or testing specific packages. +> Avoid running the full `cargo test` suite locally. The integration tests (`tests/docker_integration_tests.rs`) spawn Docker containers testing multiple versions of Bash, which is extremely slow. Prefer running `cargo test --lib` or testing specific packages. Before pushing, verify code quality with `cargo fmt --all -- --check`, `cargo clippy --workspace --all-features -- -D warnings`, and `cargo doc --workspace --no-deps --document-private-items`. ## Guidelines 1. **Safety & Stability**: `flyline` runs inside the active shell process. Avoid unwinding panics across the C FFI boundary; wrap entry points in `catch_unwind_safe` to prevent shell crashes. Never create an `App` instance in library unit tests, as `App` depends on global FFI symbols (like `history_list` or `current_readline_prompt`) that are only resolved dynamically when loaded inside Bash, causing linker failures in library test targets. diff --git a/src/active_suggestions.rs b/src/active_suggestions.rs index d04e19e5..3a453852 100644 --- a/src/active_suggestions.rs +++ b/src/active_suggestions.rs @@ -1748,13 +1748,13 @@ impl ActiveSuggestions { /// Set the selected position from a flat (1-D) suggestion index. pub fn set_selected_by_idx(&mut self, filtered_idx: usize) { - if self.last_num_rows_per_col == 0 { - self.selected_coord = Some((0, filtered_idx)); - } else { - self.selected_coord = Some(( - filtered_idx / self.last_num_rows_per_col, - filtered_idx % self.last_num_rows_per_col, - )); + match filtered_idx.checked_div(self.last_num_rows_per_col) { + Some(col) => { + self.selected_coord = Some((col, filtered_idx % self.last_num_rows_per_col)); + } + None => { + self.selected_coord = Some((0, filtered_idx)); + } } self.clamp_selection(); } diff --git a/src/agent_mode.rs b/src/agent_mode.rs index b750fb96..52a1c84e 100644 --- a/src/agent_mode.rs +++ b/src/agent_mode.rs @@ -242,10 +242,8 @@ pub(crate) fn markdown_to_text(markdown: &str, palette: &crate::palette::Palette current_spans.push(Span::raw(" ")); } } - Event::HardBreak | Event::Rule => { - if table_accum.is_none() { - finalize_line(&mut lines, &mut current_spans, list_depth); - } + Event::HardBreak | Event::Rule if table_accum.is_none() => { + finalize_line(&mut lines, &mut current_spans, list_depth); } _ => {} } diff --git a/src/app/actions/keyboard.rs b/src/app/actions/keyboard.rs index c4728ebc..268ca1f7 100644 --- a/src/app/actions/keyboard.rs +++ b/src/app/actions/keyboard.rs @@ -1960,7 +1960,7 @@ pub fn key_sequence_completer(current: &std::ffi::OsStr) -> Vec Vec ActiveSuggestionsBuilder { } } - scored.sort_by(|a, b| b.0.cmp(&a.0)); + scored.sort_by_key(|b| std::cmp::Reverse(b.0)); let res = scored.into_iter().map(|(_, info)| info).collect(); ActiveSuggestionsBuilder::from_processed(processed_suggestions_from_command_info(res)) } diff --git a/src/history/importing.rs b/src/history/importing.rs index a314ab6e..50b9cb0e 100644 --- a/src/history/importing.rs +++ b/src/history/importing.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; use std::fs::File; -use std::io::{BufRead, BufReader, Read}; +use std::io::{BufRead, Read}; use std::path::{Path, PathBuf}; use std::process::Command; diff --git a/src/perf.rs b/src/perf.rs index 129383d1..c4c9a84c 100644 --- a/src/perf.rs +++ b/src/perf.rs @@ -76,7 +76,7 @@ impl PerfRecorder { } // Sort metrics by total time ascending (shortest first, longest last) - metrics.sort_by(|a, b| a.total.cmp(&b.total)); + metrics.sort_by_key(|a| a.total); let mut report = serde_json::Map::new(); for m in metrics {