Jsonl cleanup

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.

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();
     }

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);
             }
             _ => {}
         }

src/app/actions/keyboard.rs

@@ -1960,7 +1960,7 @@ pub fn key_sequence_completer(current: &std::ffi::OsStr) -> Vec<CompletionCandid
     let current_lower = current.to_lowercase();
     let mut out = vec![];
 
-    for (_m, mod_equivs) in MODS_TO_EQUIV_NAMES.iter() {
+    for mod_equivs in MODS_TO_EQUIV_NAMES.values() {
         log::info!(
             "Checking mod_equivs {:?} against used mods {:?}",
             mod_equivs,
@@ -2013,7 +2013,7 @@ pub fn remap_key_completer(current: &std::ffi::OsStr) -> Vec<CompletionCandidate
     let mut out = key_sequence_completer(current);
 
     // 2. Also support remapping standalone modifiers (e.g. "Ctrl", "Alt", "Shift", etc.).
-    for (_, mod_equivs) in MODS_TO_EQUIV_NAMES.iter() {
+    for mod_equivs in MODS_TO_EQUIV_NAMES.values() {
         for equiv in *mod_equivs {
             if equiv.to_lowercase().starts_with(&current_lower) {
                 out.push(CompletionCandidate::new(capitalize_first(equiv)));

src/completions/tab_completion.rs

@@ -677,7 +677,7 @@ fn tab_complete_fuzzy_first_word(command: &str) -> 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))
 }

src/history/backend.rs

@@ -206,7 +206,7 @@ pub(super) struct JsonlFetchResult {
     pub(super) last_read_offset: Option<LastJsonlReadOffset>,
 }
 
-fn fetch_flyline_jsonl_history_from_offset(
+pub(super) fn fetch_flyline_jsonl_history_from_offset(
     path: &Path,
     last_offset: Option<&LastJsonlReadOffset>,
 ) -> anyhow::Result<JsonlFetchResult> {
@@ -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 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::<HistoryJsonlEvent>(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();
+    }
 
-    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);
+    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);
+    }
 }

src/history/importing.rs

@@ -1,10 +1,12 @@
 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;
 
-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,24 +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)
-        && 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()
-                && let Ok(event) = serde_json::from_str::<HistoryJsonlEvent>(trimmed)
-                && 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(

src/history/mod.rs

@@ -535,15 +535,6 @@ impl HistoryManager {
         normalized
     }
 
-    fn merge_history_entries(
-        zsh_entries: Vec<HistoryEntry>,
-        bash_entries: Vec<HistoryEntry>,
-    ) -> Vec<HistoryEntry> {
-        let mut all = zsh_entries;
-        all.extend(bash_entries);
-        Self::normalize_entries(all)
-    }
-
     #[cfg(test)]
     pub fn parse_bash_history_from_memory() -> Vec<HistoryEntry> {
         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();

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 {