VirtualStorageProvider: Make new() private, add new_physical (#764)

## Completed Changes

- [x] Made `VirtualStorageProvider::new()` fully private (not just
doc-hidden)
- [x] Updated all unit tests in virtual_storage_provider.rs to use
`new_memory()`
- [x] Updated all usages across diskann-providers, diskann-disk, and
diskann-tools to use specific constructors
- [x] Added `VirtualStorageProvider::new_physical` to clippy.toml
disallowed-methods list
- [x] Removed all unused imports (MemoryFS, OverlayFS)
- [x] Made test helper functions generic over filesystem type where
needed
- [x] **Fixed doc tests:**
  - Added missing `use std::io::Write;` import to doc examples
- Removed doc examples that write to physical filesystem (new_physical
and new_overlay)
  - Simplified to only show `new_memory()` usage
  - Removed unnecessary turbofish notation in test code
- [x] Verified all checks pass:
  - ✅ cargo fmt --all --check
  - ✅ cargo clippy --locked --workspace --all-targets --no-deps
  - ✅ cargo test --doc --workspace
  - ✅ All unit tests pass

All PR review feedback has been addressed.

<!-- START COPILOT ORIGINAL PROMPT -->



<details>

<summary>Original prompt</summary>

> 
> ----
> 
> *This section details on the original issue you should resolve*
> 
> <issue_title>VirtualStorageProvider: encourage use of `new_overlay` or
`new_memory`</issue_title>
> <issue_description>Make `VirtualStorageProvider::new` private.
> Also add VirtualStorageProvider<PhysicalFS>::new_physical<P:
AsRef<std::path::Path>>(path: P).
> Expose only new_overlay(), new_memory(), new_physical().
> 
> Examples like these should be replaced with
VirtualStorageProvider::new_memory():
>         let fs = OverlayFS::new(&[MemoryFS::default().into()]);
>         let storage_provider = VirtualStorageProvider::new(fs);
> 
> Original post:
> > Do you see any value in making `VirtualStorageProvider::new` much
harder to call to encourage use of `new_overlay` or `new_memory`? Maybe
with like a `new_physical` and then linting that with clippy?
> 
> _Originally posted by @hildebrandmw in
[#700](https://github.com/microsoft/DiskANN/pull/700/changes#r2722436904)_</issue_description>
> 
> ## Comments on the Issue (you are @copilot in this section)
> 
> <comments>
> </comments>
> 


</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #703

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in
our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: arrayka <1551741+arrayka@users.noreply.github.com>
Co-authored-by: arrayka <arrayka@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: 3 people

clippy.toml

@@ -3,6 +3,7 @@ disallowed-methods = [
     # We still have `std::fs` references.
     # { path = "std::fs", reason = "Do not use std::fs directly. Instead, use the FileStorageProvider." },
     { path = "vfs::PhysicalFS::new", reason = "Do not use vfs::PhysicalFS in tests. Instead, use the VirtualStorageProvider::new_overlay()." },
+    { path = "diskann_providers::storage::virtual_storage_provider::VirtualStorageProvider::new_physical", reason = "Do not use VirtualStorageProvider::new_physical() in tests. Instead, use VirtualStorageProvider::new_overlay()." },
     # Disallowed methods for the rayon crate to enforce execution within a specified thread pool instead of the global thread pool.
     { path = "rayon::iter::ParallelIterator::for_each", reason = "Use `for_each_in_pool` from rayon_utils.rs instead to enforce execution within a specified thread pool."},
     { path = "rayon::iter::ParallelIterator::for_each_with", reason = "Use `for_each_with_in_pool` instead to enforce execution within a specified thread pool."},

diskann-disk/src/build/builder/build.rs

@@ -916,7 +916,6 @@ mod start_point_tests {
 
     use diskann_providers::storage::VirtualStorageProvider;
     use diskann_providers::utils::write_metadata;
-    use vfs::MemoryFS;
 
     use super::*;
 
@@ -930,8 +929,7 @@ mod start_point_tests {
     #[test]
     fn test_start_point_save_and_load() {
         let file_path = "/start_point_test.bin";
-        let fs = MemoryFS::new();
-        let storage_provider = VirtualStorageProvider::new(fs);
+        let storage_provider = VirtualStorageProvider::new_memory();
 
         // Create and save a start point
         let id = 42u32;
@@ -945,7 +943,7 @@ mod start_point_tests {
 
     #[test]
     fn test_start_point_load_nonexistent_file() {
-        let storage_provider = VirtualStorageProvider::new(MemoryFS::new());
+        let storage_provider = VirtualStorageProvider::new_memory();
         let result = StartPoint::load("/nonexistent_file.bin", &storage_provider);
         assert_eq!(
             result.err().unwrap().kind(),
@@ -956,8 +954,7 @@ mod start_point_tests {
     #[test]
     fn test_start_point_load_empty_file() {
         let file_path = "/empty_file.bin";
-        let fs = MemoryFS::new();
-        let storage_provider = VirtualStorageProvider::new(fs);
+        let storage_provider = VirtualStorageProvider::new_memory();
 
         // Create an empty file
         {
@@ -972,8 +969,7 @@ mod start_point_tests {
     #[test]
     fn test_start_point_load_invalid_data() {
         let file_path = "/invalid_data.bin";
-        let fs = MemoryFS::new();
-        let storage_provider = VirtualStorageProvider::new(fs);
+        let storage_provider = VirtualStorageProvider::new_memory();
 
         // Create a file with invalid data
         {

diskann-disk/src/storage/cached_reader.rs

@@ -130,8 +130,7 @@ mod cached_reader_test {
             0x20, 0x41, 0x00, 0x00, 0x30, 0x41, 0x00, 0x00, 0x40, 0x41, 0x00, 0x00, 0x50, 0x41,
             0x00, 0x00, 0x60, 0x41, 0x00, 0x00, 0x70, 0x41, 0x00, 0x11, 0x80, 0x41,
         ];
-        let filesystem = MemoryFS::new();
-        let storage_provider = VirtualStorageProvider::new(filesystem);
+        let storage_provider = VirtualStorageProvider::new_memory();
         {
             let mut writer = storage_provider.create_for_write(file_name).unwrap();
             writer.write_all(&data).unwrap();
@@ -190,8 +189,7 @@ mod cached_reader_test {
             0x20, 0x41, 0x00, 0x00, 0x30, 0x41, 0x00, 0x00, 0x40, 0x41, 0x00, 0x00, 0x50, 0x41,
             0x00, 0x00, 0x60, 0x41, 0x00, 0x00, 0x70, 0x41, 0x00, 0x11, 0x80, 0x41,
         ];
-        let filesystem = MemoryFS::new();
-        let storage_provider = VirtualStorageProvider::new(filesystem);
+        let storage_provider = VirtualStorageProvider::new_memory();
         {
             let mut writer = storage_provider.create_for_write(file_name).unwrap();
             writer.write_all(&data).unwrap();

diskann-disk/src/storage/quant/generator.rs

@@ -283,7 +283,7 @@ mod generator_tests {
         create_thread_pool_for_test, read_metadata, save_bin_f32, save_bytes,
     };
     use rstest::rstest;
-    use vfs::{FileSystem, MemoryFS, OverlayFS};
+    use vfs::{FileSystem, MemoryFS};
 
     use super::*;
     use crate::build::chunking::continuation::{
@@ -373,11 +373,12 @@ mod generator_tests {
         dim: usize,
         offset: usize,
         output_dim: u32,
-    ) -> ANNResult<(VirtualStorageProvider<OverlayFS>, String, String)> {
-        let fs = OverlayFS::new(&[MemoryFS::default().into()]);
-        fs.create_dir("/test_data")
+    ) -> ANNResult<(VirtualStorageProvider<MemoryFS>, String, String)> {
+        let storage_provider = VirtualStorageProvider::new_memory();
+        storage_provider
+            .filesystem()
+            .create_dir("/test_data")
             .expect("Could not create test directory");
-        let storage_provider = VirtualStorageProvider::new(fs);
 
         let data_path = "/test_data/test_data.bin".to_string();
         let compressed_path = "/test_data/test_compressed.bin".to_string();
@@ -411,10 +412,10 @@ mod generator_tests {
         Ok((storage_provider, data_path, compressed_path))
     }
 
-    fn create_and_call_generator(
+    fn create_and_call_generator<F: vfs::FileSystem>(
         offset: usize,
         compressed_path: String,
-        storage_provider: &VirtualStorageProvider<OverlayFS>,
+        storage_provider: &VirtualStorageProvider<F>,
         data_path: String,
         output_dim: u32,
         chunking_config: &ChunkingConfig,

diskann-disk/src/storage/quant/pq/pq_generation.rs

@@ -194,7 +194,7 @@ mod pq_generation_tests {
     use diskann_utils::views::{MatrixView, MutMatrixView};
     use diskann_vector::distance::Metric;
     use rstest::rstest;
-    use vfs::{FileSystem, MemoryFS, OverlayFS};
+    use vfs::FileSystem;
 
     use super::{CompressionStage, PQGeneration, PQGenerationContext};
     use crate::storage::quant::compressor::QuantCompressor;
@@ -210,9 +210,9 @@ mod pq_generation_tests {
         100.0f32, 100.0f32, 100.0f32, 100.0f32, 100.0f32, 100.0f32, 100.0f32,
     ];
     #[allow(clippy::too_many_arguments)]
-    fn create_new_compressor<'a, R: AsThreadPool>(
+    fn create_new_compressor<'a, R: AsThreadPool, F: vfs::FileSystem>(
         stage: CompressionStage,
-        provider: &'a VirtualStorageProvider<OverlayFS>,
+        provider: &'a VirtualStorageProvider<F>,
         dim: usize,
         num_chunks: usize,
         max_kmeans_reps: usize,
@@ -222,7 +222,7 @@ mod pq_generation_tests {
         pivots_path: String,
         compressed_path: String,
         data_path: Option<&str>,
-    ) -> Result<PQGeneration<'a, f32, VirtualStorageProvider<OverlayFS>, R>, ANNError> {
+    ) -> Result<PQGeneration<'a, f32, VirtualStorageProvider<F>, R>, ANNError> {
         let pq_storage = PQStorage::new(&pivots_path, &compressed_path, data_path);
         let context = PQGenerationContext::<'_, _, _> {
             pq_storage,
@@ -241,10 +241,11 @@ mod pq_generation_tests {
 
     #[rstest]
     fn test_create_and_load_pivots_file() {
-        let fs = OverlayFS::new(&[MemoryFS::default().into()]);
-        fs.create_dir("/pq_generation_tests")
+        let storage_provider = VirtualStorageProvider::new_memory();
+        storage_provider
+            .filesystem()
+            .create_dir("/pq_generation_tests")
             .expect("Could not create test directory");
-        let storage_provider = VirtualStorageProvider::new(fs);
 
         let pivot_file_name = "/pq_generation_tests/generate_pq_pivots_test.bin";
         let pivot_file_name_compressor = "/pq_generation_tests/compressor_pivots_test.bin";
@@ -319,13 +320,11 @@ mod pq_generation_tests {
 
     #[rstest]
     fn throw_error_for_resume_and_no_existing_file() {
-        let fs = OverlayFS::new(&[
-            MemoryFS::default().into(),
-            // PhysicalFS::new("tests/data/").into(),
-        ]);
-        fs.create_dir("/pq_generation_tests")
+        let storage_provider = VirtualStorageProvider::new_memory();
+        storage_provider
+            .filesystem()
+            .create_dir("/pq_generation_tests")
             .expect("Could not create test directory");
-        let storage_provider = VirtualStorageProvider::new(fs);
 
         let pivot_file_name = "/pq_generation_tests/generate_pq_pivots_test.bin";
         let compressed_file_name = "/pq_generation_tests/compressed_not_used.bin";

diskann-providers/benches/benchmarks/copy_aligned_data_bench.rs

@@ -14,7 +14,6 @@ use diskann_providers::{
 };
 use rand::Rng;
 use tempfile::TempDir;
-use vfs::PhysicalFS;
 
 pub const TEST_DATA_PATH: &str = "test_aligned_data.bin";
 pub const BENCHMARK_ID: &str = "copy_aligned_data";
@@ -25,7 +24,7 @@ pub fn benchmark_copy_aligned_data(c: &mut Criterion) {
         clippy::disallowed_methods,
         reason = "Use physical file system rather than memory for testing the actual disk read/write"
     )]
-    let storage_provider = VirtualStorageProvider::new(PhysicalFS::new(tmp_dir.path()));
+    let storage_provider = VirtualStorageProvider::new_physical(tmp_dir.path());
 
     let num_points = 1_000_000;
     let num_pq_chunks = 192;

diskann-providers/benches/benchmarks_iai/copy_aligned_data_bench_iai.rs

@@ -14,7 +14,6 @@ use diskann_providers::{
 use iai_callgrind::black_box;
 use rand::Rng;
 use tempfile::TempDir;
-use vfs::PhysicalFS;
 
 pub const TEST_DATA_PATH: &str = "test_aligned_data.bin";
 pub const BENCHMARK_ID: &str = "copy_aligned_data";
@@ -31,7 +30,7 @@ pub fn benchmark_copy_aligned_data_iai() {
         clippy::disallowed_methods,
         reason = "Use physical file system rather than memory for testing the actual disk read/write"
     )]
-    let storage_provider = VirtualStorageProvider::new(PhysicalFS::new(tmp_dir.path()));
+    let storage_provider = VirtualStorageProvider::new_physical(tmp_dir.path());
 
     let num_points = 1_000_000;
     let num_pq_chunks = 192;

diskann-providers/src/model/graph/provider/async_/fast_memory_quant_vector_provider.rs

@@ -366,7 +366,6 @@ mod tests {
     use crate::storage::VirtualStorageProvider;
     use diskann::{ANNErrorKind, utils::ONE};
     use diskann_vector::{DistanceFunction, PreprocessedDistanceFunction, distance::Metric};
-    use vfs::MemoryFS;
 
     use super::*;
 
@@ -522,7 +521,7 @@ mod tests {
     fn test_direct_save_load() {
         type Provider = FastMemoryQuantVectorProviderAsync;
 
-        let storage = VirtualStorageProvider::new(MemoryFS::new());
+        let storage = VirtualStorageProvider::new_memory();
         let provider = create_test_provider();
 
         // Save to disk.
@@ -546,7 +545,7 @@ mod tests {
     async fn test_async_save_load() {
         type Provider = FastMemoryQuantVectorProviderAsync;
 
-        let storage = VirtualStorageProvider::new(MemoryFS::new());
+        let storage = VirtualStorageProvider::new_memory();
         let provider = create_test_provider();
 
         let prefix = "/data.bin";

diskann-providers/src/model/graph/provider/async_/fast_memory_vector_provider.rs

@@ -334,7 +334,6 @@ mod tests {
     use crate::storage::VirtualStorageProvider;
     use diskann::utils::vecid_from_usize;
     use diskann_vector::distance::Metric;
-    use vfs::MemoryFS;
 
     use super::*;
     use crate::test_utils::graph_data_type_utils::GraphDataF32VectorUnitData;
@@ -449,7 +448,7 @@ mod tests {
     #[test]
     fn test_direct_save_load() {
         type Provider = FastMemoryVectorProviderAsync<GraphDataF32VectorUnitData>;
-        let storage = VirtualStorageProvider::new(MemoryFS::new());
+        let storage = VirtualStorageProvider::new_memory();
         let provider = create_test_provider();
 
         // Save to disk.
@@ -472,7 +471,7 @@ mod tests {
     async fn test_async_save() {
         type Provider = FastMemoryVectorProviderAsync<GraphDataF32VectorUnitData>;
 
-        let storage = VirtualStorageProvider::new(MemoryFS::new());
+        let storage = VirtualStorageProvider::new_memory();
 
         let provider = create_test_provider();
 

diskann-providers/src/model/graph/provider/async_/inmem/scalar.rs

@@ -833,7 +833,6 @@ mod tests {
     use diskann_utils::views::MatrixView;
     use diskann_vector::distance::Metric;
     use rstest::rstest;
-    use vfs::MemoryFS;
 
     use super::*;
 
@@ -989,7 +988,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_save_with_and_load_with() {
-        let storage_provider = VirtualStorageProvider::new(MemoryFS::default());
+        let storage_provider = VirtualStorageProvider::new_memory();
         let store = make_store(Metric::InnerProduct);
 
         // Save to our memory provider