feat: persist filter state across sessions#430
Conversation
Saves all filter selections (fit, availability, TP, sort, installed-first, search query, and all multi-select popup filters) to ~/.config/llmfit/filters.json so they are restored on next launch. Follows the existing theme persistence pattern. Multi-select filters are stored by name rather than index position, so changes to the model database between runs won't corrupt saved state. Closes AlexsJones#428
AlexsJones
left a comment
AlexsJones
left a comment
There was a problem hiding this comment.
Nice work! Clean structure and good use of Option fields for forward compat. Left a few inline comments, mostly around save timing.
| if self.filtered_fits.is_empty() { | ||
| self.selected_row = 0; | ||
| } else if self.selected_row >= self.filtered_fits.len() { | ||
| self.selected_row = self.filtered_fits.len() - 1; |
There was a problem hiding this comment.
This is the only call site for save_filters(), but most filter changes go through apply_filters() directly (fit, availability, TP, search, popups). That means those changes won't actually persist.
Consider saving on quit instead (in tui_events.rs where should_quit = true). That avoids disk I/O on every keystroke too.
| sim_cursor_position: 0, | ||
| context_limit, | ||
| theme: Theme::load(), | ||
| backend_hidden_count, |
There was a problem hiding this comment.
Small thing: the old code called apply_filters() here, which was sufficient since all_fits is already sorted during construction. re_sort() re-sorts them again unnecessarily. Not a bug, just a wasted pass on startup.
| fn config_path() -> Option<PathBuf> { | ||
| let home = std::env::var("HOME") | ||
| .or_else(|_| std::env::var("USERPROFILE")) | ||
| .ok()?; |
There was a problem hiding this comment.
Looks like Theme uses the same HOME/USERPROFILE approach so this is consistent. If you ever want to align with platform conventions (~/Library/Application Support on macOS, XDG on Linux), the dirs crate handles that nicely. No blocker though.
Address PR review feedback: - Move save_filters() from apply_filters() to quit handler to avoid disk I/O on every filter change and ensure all paths persist state - Replace re_sort() with apply_filters() on startup since all_fits is already sorted during construction - Fix rustfmt formatting
|
Thanks for the review! Pushed a fix:
|
|
Hey thanks for the quick turnaround on those fixes. Sorry I didn't flag this earlier but the Other than that this looks good to go. |
Replace manual HOME/USERPROFILE env var lookup with dirs::config_dir() which handles macOS ~/Library/Application Support, Linux XDG, and Windows AppData correctly.
|
#3dbf3db Added dirs as a dependency to llmfit-tui for the config_dir() switch. Happy to open a follow-up PR to migrate theme.rs (and any other HOME/USERPROFILE call sites) to dirs as well. |
|
Opened #437 to migrate the remaining HOME/USERPROFILE call sites (theme.rs, update.rs, providers.rs) to dirs as well. |
2 participants
Summary
~/.config/llmfit/filters.jsonso they are restored on next launchTheme::save()/Theme::load())Optionfor forward/backward compatibilityTest plan
~/.config/llmfit/filters.json— verify app starts with defaultsCloses #428