Skip to content

fix(core): drive-qualify aggregate rollup and duplicate keys#575

Merged
githubrobbi merged 1 commit into
mainfrom
fix/aggregate-cross-drive-keys
Jul 22, 2026
Merged

fix(core): drive-qualify aggregate rollup and duplicate keys#575
githubrobbi merged 1 commit into
mainfrom
fix/aggregate-cross-drive-keys

Conversation

@githubrobbi

Copy link
Copy Markdown
Collaborator

Problem

Multi-drive aggregations merged per-drive identifiers as if they were global, corrupting rollup and extension-keyed results. Found live via the MCP top_folders preset returning malformed, unusable keys on a multi-drive host.

Fixes

Rollup keys (top_folders and all path/drive rollups). Group keys were bare per-drive record indices. The cross-drive merge collided buckets from different drives, and finalize resolved every key against the first drive's records — malformed folder names or record_N fallbacks, with one drive's byte totals silently folded into another's buckets. Keys now carry the drive ordinal (high 32 bits) and resolve against the drive they came from.

Duplicate composite keys. duplicates:size+ext pushed the raw per-drive extension_id: the same extension never grouped across drives, while different extensions with colliding intern ids grouped falsely. The key now hashes the interned (lowercased) extension string.

Nested rollup sub-aggregations. rollup:...,sub=terms:ext finalized without the cross-drive ExtensionMap, resolving canonical ids through a legacy per-drive fallback (wrong extension names in sub-buckets). The map is now a required parameter through the feed and finalize chains; both dead fallbacks are deleted.

Ancestor/drilldown rollups. The per-drive record_idx was applied to every scanned drive, bucketing other drives' records against arbitrary unrelated folders. RollupMode::Ancestor now names its drive (rollup:ancestor,record=N,drive=C); out-of-scope records are skipped, including in nested sub-accumulators. The wire spec gained an optional, backward-compatible drive slot; the daemon rejects ancestor specs without one (typed WireSpecError::AncestorDriveMissing).

Structure

duplicates.rs test module moved to duplicates/tests.rs (mirrors the compact_cache/tests.rs extraction) to stay under the 800 LOC policy without adding an exception.

Tests

All regression tests were verified to fail against the pre-fix code:

  • top_folders_multi_drive_keys_resolve_per_drive — pre-fix, drive D's folder vanished entirely and its bytes were credited to C:\Projects
  • extension_key_groups_across_drives_by_name_not_intern_id — pre-fix, the false avi/mp4 id-collision group survived while the real cross-drive jpg pair was missed
  • ancestor_rollup_scoped_to_named_drive — drilldown into C:\Projects buckets only drive C records with exact byte totals
  • Parser tests for the new drive= syntax and both error paths; wire conversion accept/reject tests; rollup key encode/decode round-trip and out-of-range-ordinal fallback

Gates: full workspace suite (2493 tests), lint-prod, lint-tests, rustdoc (--document-private-items --locked), doc-tests, and xwin Windows clippy all green; the full pre-push bundle passed on push.

Multi-drive aggregations merged per-drive identifiers as if they were
global, corrupting every rollup and extension-keyed result:

- Rollup group keys were bare per-drive record indices. The cross-drive
  merge collided buckets from different drives and finalize resolved
  every key against the FIRST drive's records, producing malformed
  folder names (or record_N fallbacks) from the top_folders preset.
  Keys now carry the drive ordinal (high 32 bits) and resolve against
  the drive they came from.

- Duplicate composite keys pushed the raw per-drive extension_id for
  ext key fields: the same extension never grouped across drives while
  different extensions with colliding intern ids grouped falsely. The
  key now hashes the interned (lowercased) extension string.

- Nested rollup sub-aggregations finalized without the cross-drive
  ExtensionMap, resolving canonical extension ids through a legacy
  per-drive fallback. The map is now a required parameter through the
  feed and finalize chains and both dead fallbacks are deleted.

- Ancestor/drilldown rollups applied their per-drive record index to
  every scanned drive. RollupMode::Ancestor now names its drive
  (rollup:ancestor,record=N,drive=C); records on other drives are
  skipped, including in nested sub-accumulators. The wire spec gained
  an optional backward-compatible drive slot and the daemon rejects
  ancestor specs without one.

The duplicates.rs test module moved to duplicates/tests.rs (mirroring
the compact_cache/tests.rs extraction) to stay under the 800 LOC file
policy without adding an exception.

Regression tests cover the multi-drive top_folders keys, cross-drive
extension duplicate grouping, ancestor drive scoping, parser syntax and
error paths, and wire conversion.
@githubrobbi
githubrobbi added this pull request to the merge queue Jul 22, 2026
Merged via the queue into main with commit 5c60147 Jul 22, 2026
21 checks passed
@githubrobbi
githubrobbi deleted the fix/aggregate-cross-drive-keys branch July 22, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant