From 778b55cf62c57cbd94cb784c6a514a15eee3ac3a Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Tue, 19 May 2026 09:47:49 -0700 Subject: [PATCH] refactor(uffs-core): demote DirCacheExt to free fn dir_cache_with_capacity (Phase 7b, refs #287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DirCacheExt` was a single-method `pub(crate)` extension trait providing `with_capacity(n) -> Self` over the `DirCache = FxHashMap` type alias. Per the Phase 7 trait-justification audit (playbook §879-886 J1/J2/J3/J4), it satisfied none of the four criteria: * J1 multiple impls: ❌ 1 impl (for `DirCache` only) * J2 test substitution: ❌ no test fakes * J3 extension surface: ❌ `pub(crate)`, no rustdoc claim * J4 high-level/infra decoupling: ❌ single-method ergonomic constructor The trait forced every caller to write `use … DirCacheExt as _;` just to spell `DirCache::with_capacity(n)`, and the indirection made the call site easy to misread as an inherent method on `DirCache`. This refactor replaces the trait with a free `pub(crate) fn dir_cache_with_capacity(capacity: usize) -> DirCache` and updates the 7 call sites across 4 files in `uffs-core::search::query`. Behavior preserved bit-for-bit: the new function body is the same `FxHashMap::with_capacity_and_hasher(n, FxBuildHasher)` expression that the trait impl had. API impact: zero (trait + impl were `pub(crate)`). Verification: * `cargo check -p uffs-core` — clean * `cargo clippy -p uffs-core --all-targets -- -D warnings` — clean * `cargo nextest run -p uffs-core` — 817/817 passing (3 skipped) Findings doc (local-only): docs/dev/baseline/2026-05-19/phase_7_trait_justification_findings.md records all 13 prod traits classified J1-J4; `DirCacheExt` is the only demotion. The 12 other traits (9 `uffs-daemon::cache` hooks + `FileReader` + `FormatRow` + `RuntimeDir`) are confirmed KEEP under J1 / J2 / J3. Refs: #287 --- crates/uffs-core/src/search/query/mod.rs | 6 ++--- .../src/search/query/numeric_top_n.rs | 4 ++-- .../src/search/query/path_only_top_n.rs | 6 ++--- .../src/search/query/path_sorted_top_n.rs | 6 ++--- crates/uffs-core/src/search/tree.rs | 23 +++++++++++-------- 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/crates/uffs-core/src/search/query/mod.rs b/crates/uffs-core/src/search/query/mod.rs index 8551ff862..f59c34698 100644 --- a/crates/uffs-core/src/search/query/mod.rs +++ b/crates/uffs-core/src/search/query/mod.rs @@ -21,7 +21,7 @@ use super::backend::{DisplayRow, FilterMode, PhaseTimings}; use super::field::FieldId; use super::filters::SearchFilters; use crate::compact::{CompactRecord, DriveCompactIndex}; -use crate::search::tree::{self, DirCacheExt as _}; +use crate::search::tree; /// Whether cache profiling is enabled (`UFFS_CACHE_PROFILE` env var). /// @@ -475,7 +475,7 @@ pub(crate) fn search_compact_drive_tree( let match_count = match_indices.len(); let t_resolve = std::time::Instant::now(); - let mut dir_cache = tree::DirCache::with_capacity(256); + let mut dir_cache = tree::dir_cache_with_capacity(256); let rows: Vec = match_indices .iter() .filter_map(|&record_idx| { @@ -584,7 +584,7 @@ fn indices_to_rows( indices: &[u32], volume_prefix: &str, ) -> Vec { - let mut dir_cache = tree::DirCache::with_capacity(256); + let mut dir_cache = tree::dir_cache_with_capacity(256); indices .iter() .filter_map(|&record_idx| { diff --git a/crates/uffs-core/src/search/query/numeric_top_n.rs b/crates/uffs-core/src/search/query/numeric_top_n.rs index 06ae8c0a9..94bc8380e 100644 --- a/crates/uffs-core/src/search/query/numeric_top_n.rs +++ b/crates/uffs-core/src/search/query/numeric_top_n.rs @@ -11,7 +11,7 @@ use super::super::backend::{self, DisplayRow, FilterMode, PhaseTimings}; use super::super::derived::bulkiness_for_record; use super::super::field::FieldId; use super::super::filters::SearchFilters; -use super::super::tree::{self, DirCacheExt as _}; +use super::super::tree; use super::{HeapEntry, heap_push_capped, make_display_row, stack_volume_prefix}; use crate::compact::{CompactRecord, DriveCompactIndex}; @@ -528,7 +528,7 @@ fn resolve_chunk>( let volume_prefix = stack_volume_prefix(&mut vp_buf, drive.letter); let cache = local_caches .entry(drive_idx) - .or_insert_with(|| tree::DirCache::with_capacity(256)); + .or_insert_with(|| tree::dir_cache_with_capacity(256)); let t_resolve = std::time::Instant::now(); let path = tree::resolve_path_cached(drive, rec_idx as usize, volume_prefix, cache); resolve_fn_ns += t_resolve.elapsed().as_nanos(); diff --git a/crates/uffs-core/src/search/query/path_only_top_n.rs b/crates/uffs-core/src/search/query/path_only_top_n.rs index d1866ca3b..60a2a9dba 100644 --- a/crates/uffs-core/src/search/query/path_only_top_n.rs +++ b/crates/uffs-core/src/search/query/path_only_top_n.rs @@ -63,7 +63,7 @@ use rayon::prelude::*; use super::super::backend::{self, DisplayRow, FilterMode, PhaseTimings}; use super::super::field::FieldId; use super::super::filters::{SearchFilters, row_passes_filters}; -use super::super::tree::{self, DirCache, DirCacheExt as _}; +use super::super::tree::{self, DirCache}; use super::numeric_top_n::sort_indices_by_name; use super::{make_display_row, passes_filter_mode, stack_volume_prefix}; use crate::compact::DriveCompactIndex; @@ -160,7 +160,7 @@ pub(super) fn collect_path_only_sorted_top_n + Sync> let drive = drive_ref.as_ref(); let mut vp_buf = [0_u8; 4]; let volume_prefix = stack_volume_prefix(&mut vp_buf, drive.letter); - let mut dir_cache = DirCache::with_capacity(256); + let mut dir_cache = tree::dir_cache_with_capacity(256); // Roots = records whose parent is `u32::MAX` (typically the // drive root "." at FRS 5, though stray orphans are possible). @@ -587,7 +587,7 @@ fn collect_path_only_via_ext_index + Sync>( let volume_prefix = stack_volume_prefix(&mut vp_buf, drive.letter); let cache = local_caches .entry(drive_idx) - .or_insert_with(|| DirCache::with_capacity(256)); + .or_insert_with(|| tree::dir_cache_with_capacity(256)); let path = tree::resolve_path_cached(drive, rec_idx as usize, volume_prefix, cache); local_rows.push(make_display_row(rec_idx, drive.letter, rec, name, path)); local_candidates += 1; diff --git a/crates/uffs-core/src/search/query/path_sorted_top_n.rs b/crates/uffs-core/src/search/query/path_sorted_top_n.rs index 04c7d72b2..28997fe69 100644 --- a/crates/uffs-core/src/search/query/path_sorted_top_n.rs +++ b/crates/uffs-core/src/search/query/path_sorted_top_n.rs @@ -31,7 +31,7 @@ use rayon::prelude::*; use super::super::backend::{self, DisplayRow, FilterMode}; use super::super::field::FieldId; use super::super::filters::{SearchFilters, row_passes_filters}; -use super::super::tree::{self, DirCache, DirCacheExt as _}; +use super::super::tree::{self, DirCache}; use super::numeric_top_n::sort_indices_by_name; use super::{make_display_row, passes_filter_mode, stack_volume_prefix}; use crate::compact::DriveCompactIndex; @@ -145,7 +145,7 @@ fn walk_tree_path_sorted>( .collect(); sort_indices_by_name(&mut roots, drive, sort_desc); - let mut dir_cache = DirCache::with_capacity(256); + let mut dir_cache = tree::dir_cache_with_capacity(256); let mut stack: Vec = roots.into_iter().rev().collect(); while let Some(idx) = stack.pop() { if path_results.len() >= limit { @@ -319,7 +319,7 @@ fn collect_path_via_ext_index + Sync>( let volume_prefix = stack_volume_prefix(&mut vp_buf, drive.letter); let cache = local_caches .entry(drive_idx) - .or_insert_with(|| DirCache::with_capacity(256)); + .or_insert_with(|| tree::dir_cache_with_capacity(256)); let path = tree::resolve_path_cached(drive, rec_idx as usize, volume_prefix, cache); local_rows.push(make_display_row(rec_idx, drive.letter, rec, name, path)); } diff --git a/crates/uffs-core/src/search/tree.rs b/crates/uffs-core/src/search/tree.rs index 9999047df..fd7489177 100644 --- a/crates/uffs-core/src/search/tree.rs +++ b/crates/uffs-core/src/search/tree.rs @@ -20,16 +20,19 @@ use crate::compact::DriveCompactIndex; /// walks. Uses `FxHashMap` (3–5× faster than `HashMap` for integer keys). pub type DirCache = FxHashMap; -/// Trait extension to add `with_capacity` to `DirCache` (`FxHashMap`). -pub(crate) trait DirCacheExt { - /// Create a new `DirCache` with the given capacity. - fn with_capacity(capacity: usize) -> Self; -} - -impl DirCacheExt for DirCache { - fn with_capacity(capacity: usize) -> Self { - Self::with_capacity_and_hasher(capacity, FxBuildHasher) - } +/// Construct a [`DirCache`] with the given capacity, pre-wired with the +/// crate's `FxBuildHasher`. +/// +/// Replaces the former `DirCacheExt::with_capacity` extension trait +/// (Phase 7b refactor playbook §879-886). The trait satisfied none of +/// the J1/J2/J3/J4 justification criteria — single impl, no test fakes, +/// `pub(crate)` (no extension surface), no infrastructure decoupling — +/// and forced every caller to bring a `DirCacheExt as _` import into +/// scope just to write `DirCache::with_capacity(n)`. A free function +/// carries the same intent without the implicit trait import. +#[must_use] +pub(crate) fn dir_cache_with_capacity(capacity: usize) -> DirCache { + DirCache::with_capacity_and_hasher(capacity, FxBuildHasher) } /// Resolve a record's full path by walking the parent chain in the compact