From 0a37f3a4920099a8b36c37172f919a862e373e49 Mon Sep 17 00:00:00 2001 From: gustav-fff <286169375+gustav-fff@users.noreply.github.com> Date: Sat, 5 Sep 2026 07:02:34 -0700 Subject: [PATCH] fix(fff-mcp): apply --max-cached-files as a cap, not a repo size (#847) --max-cached-files / FFF_MAX_CACHED_FILES was passed to ContentCacheBudget::new_for_repo, which reads its argument as an indexed file count and returns a bucketed heuristic. Every value <= 10_000 came back as max_files = 30_000, so the explicit cap was silently discarded. Route it through ContentCacheBudget::with_max_files, which applies the value verbatim and keeps the default byte caps. 0 now means no persistent caching; grep still works through temporary mmaps. Refs #847 --- crates/fff-core/src/types.rs | 45 +++++++++++++++ .../fff-core/tests/explicit_cache_budget.rs | 56 +++++++++++++++++++ crates/fff-mcp/src/main.rs | 5 +- 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 crates/fff-core/tests/explicit_cache_budget.rs diff --git a/crates/fff-core/src/types.rs b/crates/fff-core/src/types.rs index 72661c967..4db0710c3 100644 --- a/crates/fff-core/src/types.rs +++ b/crates/fff-core/src/types.rs @@ -966,6 +966,16 @@ impl ContentCacheBudget { } } + /// Apply an explicit file cap verbatim, keeping the default byte caps. + /// `0` means no persistent caching at all — files stay searchable through + /// the temporary mmaps that grep releases after each call. + pub fn with_max_files(max_files: usize) -> Self { + Self { + max_files, + ..Self::default() + } + } + /// Build a budget from caller-supplied overrides. /// /// Each argument is a cap; `0` means "use the library default for that @@ -1002,3 +1012,38 @@ impl Default for ContentCacheBudget { Self::new_for_repo(30_000) } } + +#[cfg(test)] +mod content_cache_budget_tests { + use super::*; + + #[test] + fn with_max_files_applies_the_cap_verbatim() { + // regression: the cap used to be routed through new_for_repo, which + // read it as a repo file count and bucketed 2000 up to 30_000 + assert_eq!(ContentCacheBudget::with_max_files(2000).max_files, 2000); + assert_eq!(ContentCacheBudget::with_max_files(7).max_files, 7); + assert_eq!( + ContentCacheBudget::with_max_files(1_000_000).max_files, + 1_000_000 + ); + } + + #[test] + fn with_max_files_zero_disables_persistent_caching_but_keeps_grep() { + let budget = ContentCacheBudget::with_max_files(0); + assert_eq!(budget.max_files, 0); + assert!(budget.is_exhausted()); + // temporary-mmap grep is gated on max_file_size, which must survive + assert_eq!(budget.max_file_size, MAX_FFFILE_SIZE); + assert!(budget.max_bytes > 0); + } + + #[test] + fn with_max_files_keeps_default_byte_caps() { + let budget = ContentCacheBudget::with_max_files(2000); + let default = ContentCacheBudget::default(); + assert_eq!(budget.max_bytes, default.max_bytes); + assert_eq!(budget.max_file_size, default.max_file_size); + } +} diff --git a/crates/fff-core/tests/explicit_cache_budget.rs b/crates/fff-core/tests/explicit_cache_budget.rs new file mode 100644 index 000000000..0e6d2167d --- /dev/null +++ b/crates/fff-core/tests/explicit_cache_budget.rs @@ -0,0 +1,56 @@ +//! Regression test for https://github.com/dmtrKovalenko/fff/issues/847 +//! +//! An explicit `--max-cached-files` cap must reach the picker verbatim and +//! survive the initial scan, which otherwise auto-sizes the budget. + +use std::fs; + +use fff_search::file_picker::FilePicker; +use fff_search::{ContentCacheBudget, FilePickerOptions}; +use tempfile::TempDir; + +#[test] +fn explicit_cap_reaches_the_picker_and_survives_the_scan() { + let dir = TempDir::new().unwrap(); + for i in 0..8 { + fs::write(dir.path().join(format!("f{i}.txt")), "x".repeat(32 * 1024)).unwrap(); + } + + let mut picker = FilePicker::new(FilePickerOptions { + base_path: dir.path().to_string_lossy().to_string(), + watch: false, + cache_budget: Some(ContentCacheBudget::with_max_files(2)), + ..Default::default() + }) + .expect("failed to create FilePicker"); + + assert!(picker.has_explicit_cache_budget()); + assert_eq!(picker.cache_budget().max_files, 2); + + picker.collect_files().expect("failed to collect files"); + + // 8 files would otherwise bucket into the 30_000 heuristic + assert_eq!(picker.cache_budget().max_files, 2); + assert_eq!( + picker.cache_budget().max_file_size, + ContentCacheBudget::default().max_file_size + ); +} + +#[test] +fn zero_cap_keeps_the_budget_exhausted_after_the_scan() { + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("a.txt"), "x".repeat(32 * 1024)).unwrap(); + + let mut picker = FilePicker::new(FilePickerOptions { + base_path: dir.path().to_string_lossy().to_string(), + watch: false, + cache_budget: Some(ContentCacheBudget::with_max_files(0)), + ..Default::default() + }) + .expect("failed to create FilePicker"); + picker.collect_files().expect("failed to collect files"); + + assert_eq!(picker.cache_budget().max_files, 0); + assert!(picker.cache_budget().is_exhausted()); +} diff --git a/crates/fff-mcp/src/main.rs b/crates/fff-mcp/src/main.rs index 3b13f9658..f671a632e 100644 --- a/crates/fff-mcp/src/main.rs +++ b/crates/fff-mcp/src/main.rs @@ -154,7 +154,8 @@ pub(crate) struct Args { /// Maximum number of files whose content is kept persistently in memory. /// Files beyond this limit are still searchable via temporary mmaps that - /// are released after each grep. Defaults to 30 000. + /// are released after each grep. `0` disables persistent caching entirely. + /// Unset: auto-sized from the scanned file count. /// Also settable via the FFF_MAX_CACHED_FILES environment variable. #[arg(long = "max-cached-files", env = "FFF_MAX_CACHED_FILES")] max_cached_files: Option, @@ -347,7 +348,7 @@ async fn main() -> Result<(), Box> { mode: FFFMode::Ai, cache_budget: args .max_cached_files - .map(fff::ContentCacheBudget::new_for_repo), + .map(fff::ContentCacheBudget::with_max_files), follow_symlinks: args.follow_symlinks, enable_home_dir_scanning: args.enable_home_scan, enable_fs_root_scanning: args.enable_root_scan,