feat: redundancy awareness & cross-volume file comparison#3053
Conversation
Surface content redundancy data so users can answer "if this drive dies, what do I lose?" — builds on existing content identity and volume systems. Backend: - New `redundancy.summary` library query with per-volume at-risk vs redundant byte/file counts and a library-wide replication score - Extend `SearchFilters` with `at_risk`, `on_volumes`, `not_on_volumes`, `min_volume_count`, `max_volume_count` filters - Add composite index migration on entries(content_id, volume_id) Frontend: - `/redundancy` dashboard with replication score, volume bars, at-risk callout - `/redundancy/at-risk` paginated file list sorted by size - `/redundancy/compare` two-volume comparison (unique/shared toggle) - Sidebar ShieldCheck button linking to redundancy view Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview Extends Introduces a new Reviewed by Cursor Bugbot for commit 1eea61b. Configure here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end redundancy features: DB migration for a composite entries index, backend redundancy.summary query and search filters, CLI and frontend UI (dashboard, at‑risk, compare) with explorer filtered mode, volume visibility/capacity fixes, build tooling, docs, and related tests/hooks. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend Client
participant API as Backend Query Handler
participant DB as Database
User->>Frontend: open /redundancy (dashboard)
Frontend->>API: request "redundancy.summary" (input: volume_uuids?)
API->>DB: query per-volume at-risk (content on exactly 1 visible volume)
DB-->>API: at-risk rows
API->>DB: query per-volume redundant (content on >=2 volumes)
DB-->>API: redundant rows
API->>DB: query per-volume totals (file counts & bytes)
DB-->>API: totals rows
API->>DB: query library unique content bytes (deduped identities)
DB-->>API: library totals
API->>API: merge rows, compute replication_score, assemble output
API-->>Frontend: RedundancySummaryOutput
Frontend->>Frontend: render dashboard, bars, navigation (at-risk/compare)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
|
|
||
| /// Filter to files whose content is present on the specified volumes | ||
| pub fn on_volumes(mut self, on_volumes: &Option<Vec<Uuid>>) -> Self { | ||
| if let Some(uuids) = on_volumes { |
There was a problem hiding this comment.
on_volumes currently matches content present on any of the provided volumes. The compare view’s “Shared” mode (and the docstring) reads more like “present on all selected volumes”. If that’s the intent, you can enforce it with a GROUP BY + HAVING COUNT(DISTINCT ...) = uuids.len()
| if let Some(uuids) = on_volumes { | |
| pub fn on_volumes(mut self, on_volumes: &Option<Vec<Uuid>>) -> Self { | |
| if let Some(uuids) = on_volumes { | |
| if !uuids.is_empty() { | |
| let uuid_list = uuids | |
| .iter() | |
| .map(|u| format!("'{}'", u)) | |
| .collect::<Vec<_>>() | |
| .join(","); | |
| let required_count = uuids.len(); | |
| self.condition = self.condition.add(Expr::cust(format!( | |
| "entries.content_id IN (\\ | |
| SELECT e2.content_id FROM entries e2 \\ | |
| INNER JOIN volumes v ON e2.volume_id = v.id \\ | |
| WHERE e2.content_id IS NOT NULL \\ | |
| AND v.uuid IN ({}) \\ | |
| GROUP BY e2.content_id \\ | |
| HAVING COUNT(DISTINCT v.uuid) = {}\\ | |
| )", | |
| uuid_list, | |
| required_count | |
| ))); | |
| } | |
| } | |
| self | |
| } |
| if self.query.trim().is_empty() && !is_recents_query { | ||
| // Allow empty queries when redundancy filters are active (browsing at-risk files) | ||
| let has_redundancy_filters = self.filters.at_risk.is_some() | ||
| || self.filters.on_volumes.is_some() |
There was a problem hiding this comment.
Minor edge case: has_redundancy_filters treats Some(vec![]) as active, so an empty query becomes valid even though the filter builder ignores empty lists. Probably want to treat empty vectors as inactive.
| || self.filters.on_volumes.is_some() | |
| let has_redundancy_filters = self.filters.at_risk.is_some() | |
| || self | |
| .filters | |
| .on_volumes | |
| .as_ref() | |
| .map_or(false, |v| !v.is_empty()) | |
| || self | |
| .filters | |
| .not_on_volumes | |
| .as_ref() | |
| .map_or(false, |v| !v.is_empty()) | |
| || self.filters.min_volume_count.is_some() | |
| || self.filters.max_volume_count.is_some(); |
| case "unique_b": | ||
| return { ...NULL_FILTERS, on_volumes: [volumeB], not_on_volumes: [volumeA] }; | ||
| case "shared": | ||
| return { ...NULL_FILTERS, on_volumes: [volumeA, volumeB], min_volume_count: 2 }; |
There was a problem hiding this comment.
“Shared” mode currently uses on_volumes: [A, B] + min_volume_count: 2. If on_volumes is “any-of” (as implemented today), this can include files on A + C (not B). Might be worth either making on_volumes an “all-of” filter for multi-select, or introducing a dedicated shared-between-volumes filter.
|
|
||
| // Helper to build volume ID WHERE clause | ||
| let volume_where = match &volume_id_filter { | ||
| Some(ids) => { |
There was a problem hiding this comment.
When volume_uuids is provided, the outer queries are scoped but the inner COUNT(DISTINCT volume_id) subqueries aren’t, so “at risk”/“redundant” is computed globally rather than within the selected set. Also, summing ci.total_size per entry will multiply bytes when the same content_id appears multiple times on a volume. Might be worth clarifying the intended semantics (per-entry vs per-content) and, if needed, deduping (volume_id, content_id) before summing.
| /// Display name of the volume | ||
| pub display_name: Option<String>, | ||
| /// Total bytes of file content on this volume (deduplicated within volume) | ||
| pub total_bytes: i64, |
There was a problem hiding this comment.
Nit: total_bytes is documented as “deduplicated within volume”, but totals_sql sums entries.size per entry. If per-entry is intended, updating the comment avoids confusion.
| pub total_bytes: i64, | |
| /// Total bytes of file entries on this volume |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
| let total_file_count = total.map_or(0, |r| r.total_file_count as u32); | ||
|
|
||
| lib_at_risk_bytes += at_risk_bytes; | ||
| lib_redundant_bytes += redundant_bytes; |
There was a problem hiding this comment.
Library redundant bytes double-counted across volumes
High Severity
lib_redundant_bytes is computed by summing per-volume redundant bytes, but redundant content by definition exists on 2+ volumes. This means the same content's size is counted once per volume it appears on, inflating total_redundant_bytes and the replication_score. For example, a 100 MB file on 3 volumes contributes 300 MB instead of 100 MB. The doc on LibraryRedundancyTotals.total_redundant_bytes says "content bytes that exist on two or more volumes" (implying deduplicated), but the value is a per-volume sum. The replication_score is similarly overstated.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
| case "unique_b": | ||
| return { ...NULL_FILTERS, on_volumes: [volumeB], not_on_volumes: [volumeA] }; | ||
| case "shared": | ||
| return { ...NULL_FILTERS, on_volumes: [volumeA, volumeB], min_volume_count: 2 }; |
There was a problem hiding this comment.
Shared comparison mode uses OR instead of AND
High Severity
The "shared" comparison mode builds on_volumes: [volumeA, volumeB], min_volume_count: 2. The on_volumes filter uses SQL IN which matches content on volume A or B. Combined with min_volume_count: 2 (a global check across all volumes), this incorrectly includes files that exist on A and some unrelated volume C but not B. The intent is to find files shared between A and B specifically, but the filter combination doesn't express that constraint.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
| format!("AND e.volume_id IN ({})", id_list) | ||
| } | ||
| None => String::new(), | ||
| }; |
There was a problem hiding this comment.
Empty volume ID list generates invalid SQL
Medium Severity
When volume_uuids contains UUIDs that don't match any existing volumes, the resolved volume_id_filter becomes Some(vec![]). The volume_where clause then generates AND e.volume_id IN () — an empty IN list is a SQL syntax error in SQLite, causing all three subsequent queries to fail at runtime.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
| const sizes = ["B", "KB", "MB", "GB", "TB", "PB"]; | ||
| const i = Math.floor(Math.log(bytes) / Math.log(k)); | ||
| return `${(bytes / Math.pow(k, i)).toFixed(1)} ${sizes[i]}`; | ||
| } |
There was a problem hiding this comment.
Duplicated formatBytes utility across new files
Low Severity
An identical formatBytes function is defined in four new files (plus RedundancyVolumeBar.tsx). An exported formatBytes already exists at packages/interface/src/routes/explorer/utils.ts and is used by other components. The new copies are redundant and risk diverging in behavior.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
| .file_types(&self.input.filters.file_types) | ||
| .date_range(&self.input.filters.date_range) | ||
| .size_range(&self.input.filters.size_range); | ||
| .size_range(&self.input.filters.size_range) |
There was a problem hiding this comment.
Redundancy filters missing from total count query
Medium Severity
The apply_filters method used by get_total_count does not include the five new redundancy filters (at_risk, on_volumes, not_on_volumes, min_volume_count, max_volume_count), even though FilterBuilder in execute_fast_search_no_fts does. This means total_found in the response ignores redundancy filters, returning an inflated count. The at-risk files view and compare view rely on total_found for pagination, so they'll display incorrect page counts and allow navigating to empty pages.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1eea61b. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/interface/src/router.tsx (1)
4-6: Lazy-load the new redundancy route components.These components are eagerly imported on Lines 4–6, which pulls redundancy pages into the initial bundle. Prefer
React.lazy+Suspenseat route boundaries.♻️ Suggested refactor
+import { lazy, Suspense } from "react"; import { createBrowserRouter, Navigate, Outlet } from "react-router-dom"; import { Overview } from "./routes/overview"; import { ExplorerView } from "./routes/explorer"; -import { RedundancyDashboard } from "./routes/redundancy"; -import { AtRiskFiles } from "./routes/redundancy/at-risk"; -import { CompareVolumes } from "./routes/redundancy/compare"; + +const RedundancyDashboard = lazy(() => + import("./routes/redundancy").then((m) => ({ default: m.RedundancyDashboard })) +); +const AtRiskFiles = lazy(() => + import("./routes/redundancy/at-risk").then((m) => ({ default: m.AtRiskFiles })) +); +const CompareVolumes = lazy(() => + import("./routes/redundancy/compare").then((m) => ({ default: m.CompareVolumes })) +);{ path: "redundancy", children: [ { index: true, - element: <RedundancyDashboard />, + element: ( + <Suspense fallback={null}> + <RedundancyDashboard /> + </Suspense> + ), }, { path: "at-risk", - element: <AtRiskFiles />, + element: ( + <Suspense fallback={null}> + <AtRiskFiles /> + </Suspense> + ), }, { path: "compare", - element: <CompareVolumes />, + element: ( + <Suspense fallback={null}> + <CompareVolumes /> + </Suspense> + ), }, ], },As per coding guidelines: "Lazy load route components using React.lazy and Suspense for code splitting".
Also applies to: 84-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/router.tsx` around lines 4 - 6, The three redundancy route components (RedundancyDashboard, AtRiskFiles, CompareVolumes) are eagerly imported; replace those static imports in router.tsx with React.lazy(...) and wrap the route rendering boundaries in a React.Suspense fallback to enable code-splitting; specifically, change the import sites for RedundancyDashboard, AtRiskFiles and CompareVolumes to lazy-loaded declarations and ensure the Routes (where those components are used, also noted around lines 84-100) are wrapped with a Suspense element (or each route uses a Suspense boundary) so the components load asynchronously with a fallback UI.packages/interface/src/routes/redundancy/compare.tsx (2)
268-289: Consider usingclsxfor conditional class application.Per coding guidelines, prefer
clsxutility for conditional className instead of manual template literals.♻️ Refactor using clsx
+import clsx from "clsx"; function ModeButton({ active, onClick, label, }: { active: boolean; onClick: () => void; label: string; }) { return ( <button onClick={onClick} - className={`flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors ${ - active - ? "bg-accent text-white" - : "text-ink-dull hover:text-ink" - }`} + className={clsx( + "flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors", + active ? "bg-accent text-white" : "text-ink-dull hover:text-ink" + )} > {label} </button> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 268 - 289, The ModeButton component uses a template literal to conditionally set className; replace it with clsx by importing clsx and calling it in the className prop (keep static classes like "flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors" and apply the conditional object or array form for active ? "bg-accent text-white" : "text-ink-dull hover:text-ink"). Update the ModeButton function signature to use className={clsx(...)} and ensure clsx is added to the module imports.
69-83: Consider adding pagination or a "View All" link for large result sets.The query is limited to 50 results without pagination controls. For volumes with many unique or shared files, users won't see the complete picture. Consider either adding pagination (like
at-risk.tsx) or linking to the at-risk view with appropriate filters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 69 - 83, The search call using useLibraryQuery currently limits results to 50 (pagination: { limit: 50, offset: 0 }) and is gated by filters, which hides large result sets; update the component to either implement pagination state and controls (e.g., page/offset and limit state passed into the pagination object of useLibraryQuery, showing next/prev or page numbers and reflecting searchLoading/searchData) or add a "View All" / "Open At-Risk" link that navigates to the at-risk view with the current filters applied; reference useLibraryQuery, searchData/searchLoading, filters, NULL_FILTERS and the pagination object when making the changes so the query updates as users paginate or follow the link.packages/interface/src/routes/redundancy/at-risk.tsx (1)
15-22: Consider extractingformatBytesto a shared utility.This helper is duplicated in
at-risk.tsx,compare.tsx,index.tsx, andRedundancyVolumeBar.tsx. Extracting it to a shared utilities file would reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/at-risk.tsx` around lines 15 - 22, The function formatBytes is duplicated across at-risk.tsx, compare.tsx, index.tsx, and RedundancyVolumeBar.tsx; extract it into a shared utility (e.g., create a new helper like formatBytes in a utils/bytes.ts or similar), export it, and replace the local implementations by importing and using the shared formatBytes function in each file (ensure the exported function signature remains formatBytes(bytes: number): string and update imports in the components that currently define it).core/src/ops/search/input.rs (1)
206-213: Consider validatingmin_volume_count≤max_volume_countwhen both are provided.The validation logic correctly allows empty queries when redundancy filters are active, consistent with the
execute_fast_search_no_ftsimplementation. However, similar to the date_range and size_range validations below, you may want to ensuremin_volume_countdoesn't exceedmax_volume_countwhen both are specified.♻️ Optional validation addition
if let (Some(min), Some(max)) = (size_range.min, size_range.max) { if min > max { return Err("Size range min must be less than max".to_string()); } } } + // Validate volume count range if both are provided + if let (Some(min), Some(max)) = (self.filters.min_volume_count, self.filters.max_volume_count) { + if min > max { + return Err("Volume count min must be less than or equal to max".to_string()); + } + } + Ok(())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/search/input.rs` around lines 206 - 213, Add a validation that ensures self.filters.min_volume_count ≤ self.filters.max_volume_count when both are Some: locate the same validation area that checks date_range/size_range (near has_redundancy_filters / the empty-query check) and, if both self.filters.min_volume_count and self.filters.max_volume_count are present, return/emit the same kind of validation error used elsewhere (or set the same invalid flag) when min > max so the volume-count range is consistent with the other range checks.core/src/ops/redundancy/summary/query.rs (2)
254-259: Potential truncation when castingi64tou32.The
file_countandtotal_file_countvalues from SQL (i64) are cast tou32usingas. While exceeding ~4 billion files is unlikely, theascast silently truncates/wraps on overflow, which could produce incorrect results without warning.Consider using
try_into()for defensive error handling or switching the output struct fields toi64/u64.♻️ Suggested defensive cast
- let at_risk_file_count = at_risk.map_or(0, |r| r.file_count as u32); + let at_risk_file_count = at_risk.map_or(0, |r| r.file_count.try_into().unwrap_or(u32::MAX)); - let redundant_file_count = redundant.map_or(0, |r| r.file_count as u32); + let redundant_file_count = redundant.map_or(0, |r| r.file_count.try_into().unwrap_or(u32::MAX)); - let total_file_count = total.map_or(0, |r| r.total_file_count as u32); + let total_file_count = total.map_or(0, |r| r.total_file_count.try_into().unwrap_or(u32::MAX));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/redundancy/summary/query.rs` around lines 254 - 259, The current casts like in at_risk_file_count, redundant_file_count, and total_file_count use `as` to convert SQL i64 counts to u32 which silently truncates on overflow; update the conversion to perform a fallible conversion (e.g. use try_into()/TryFrom) and propagate or handle errors (return a Result or clamp/validate) or alternatively change the output fields to a wider signed/unsigned type (i64/u64) so no lossy cast is needed—locate the assignments for at_risk_file_count, redundant_file_count, and total_file_count in query.rs and implement the chosen defensive conversion/field type change consistently across the summary struct and callers.
139-143: HardcodedDbBackend::Sqlitelimits database portability.All raw SQL statements use
DbBackend::Sqlite. If Spacedrive intends to remain SQLite-only, this is fine. Otherwise, consider parameterizing the backend or adding a comment noting this constraint.Also applies to: 168-172, 189-194, 206-211
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/redundancy/summary/query.rs` around lines 139 - 143, The SQL statement construction hardcodes DbBackend::Sqlite (seen in Statement::from_string(DbBackend::Sqlite, at_risk_sql)) which breaks portability; replace the hardcoded backend with the actual database backend used at runtime (e.g., obtain the backend from the active connection or a passed context) and pass that to Statement::from_string for all occurrences (see VolumeRedundancyRow::find_by_statement usages and the at_risk_sql/other SQL variables at lines referenced). If the project is intentionally SQLite-only, instead add a clear comment next to Statement::from_string(DbBackend::Sqlite, ...) stating that SQLite is required and why; otherwise modify the calling code to supply db.backend() or an injected Backend enum so Statement::from_string uses the correct backend for each query.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/ops/redundancy/summary/query.rs`:
- Around line 196-211: The library_total_sql query (used to compute
total_unique_content_bytes via LibraryTotalRow::find_by_statement) is missing
the volume filter used elsewhere (volume_where), causing totals to be
library-wide while total_at_risk_bytes and total_redundant_bytes are filtered;
update the library_total_sql subquery on entries (the SELECT DISTINCT
e.content_id FROM entries e WHERE ...) to include the same volume_where
condition (or the equivalent e.volume_id IN (...) predicate) so
total_unique_content_bytes respects the requested volume_uuids, or alternatively
add a clear comment next to library_total_sql explaining the intentional
difference if the library-wide behavior is desired.
In `@core/src/ops/search/query.rs`:
- Around line 927-932: The redundancy filters (size_range, at_risk, on_volumes,
not_on_volumes, min_volume_count, max_volume_count) are only applied in
execute_fast_search_no_fts causing divergence; update execute_fast_search and
get_total_count to apply the same predicates. Best fix: extract the repeated
chain into a helper (e.g., apply_redundancy_filters) that accepts the query
builder and &self.input.filters and call it from execute_fast_search,
execute_fast_search_no_fts, and get_total_count so both FTS and non-FTS paths
and total-count calculations share identical filter logic.
In `@packages/interface/src/routes/redundancy/at-risk.tsx`:
- Around line 26-34: In AtRiskFiles, reset the pagination offset whenever the
URL filter parameters change: add a useEffect in the AtRiskFiles component that
calls setOffset(0) when either volumeFilter or isAtRisk changes so the component
does not retain a stale offset; ensure the effect’s dependency array includes
volumeFilter and isAtRisk (referencing the existing useState offset/setOffset
and the volumeFilter/isAtRisk variables) and avoid adding unrelated
dependencies.
---
Nitpick comments:
In `@core/src/ops/redundancy/summary/query.rs`:
- Around line 254-259: The current casts like in at_risk_file_count,
redundant_file_count, and total_file_count use `as` to convert SQL i64 counts to
u32 which silently truncates on overflow; update the conversion to perform a
fallible conversion (e.g. use try_into()/TryFrom) and propagate or handle errors
(return a Result or clamp/validate) or alternatively change the output fields to
a wider signed/unsigned type (i64/u64) so no lossy cast is needed—locate the
assignments for at_risk_file_count, redundant_file_count, and total_file_count
in query.rs and implement the chosen defensive conversion/field type change
consistently across the summary struct and callers.
- Around line 139-143: The SQL statement construction hardcodes
DbBackend::Sqlite (seen in Statement::from_string(DbBackend::Sqlite,
at_risk_sql)) which breaks portability; replace the hardcoded backend with the
actual database backend used at runtime (e.g., obtain the backend from the
active connection or a passed context) and pass that to Statement::from_string
for all occurrences (see VolumeRedundancyRow::find_by_statement usages and the
at_risk_sql/other SQL variables at lines referenced). If the project is
intentionally SQLite-only, instead add a clear comment next to
Statement::from_string(DbBackend::Sqlite, ...) stating that SQLite is required
and why; otherwise modify the calling code to supply db.backend() or an injected
Backend enum so Statement::from_string uses the correct backend for each query.
In `@core/src/ops/search/input.rs`:
- Around line 206-213: Add a validation that ensures
self.filters.min_volume_count ≤ self.filters.max_volume_count when both are
Some: locate the same validation area that checks date_range/size_range (near
has_redundancy_filters / the empty-query check) and, if both
self.filters.min_volume_count and self.filters.max_volume_count are present,
return/emit the same kind of validation error used elsewhere (or set the same
invalid flag) when min > max so the volume-count range is consistent with the
other range checks.
In `@packages/interface/src/router.tsx`:
- Around line 4-6: The three redundancy route components (RedundancyDashboard,
AtRiskFiles, CompareVolumes) are eagerly imported; replace those static imports
in router.tsx with React.lazy(...) and wrap the route rendering boundaries in a
React.Suspense fallback to enable code-splitting; specifically, change the
import sites for RedundancyDashboard, AtRiskFiles and CompareVolumes to
lazy-loaded declarations and ensure the Routes (where those components are used,
also noted around lines 84-100) are wrapped with a Suspense element (or each
route uses a Suspense boundary) so the components load asynchronously with a
fallback UI.
In `@packages/interface/src/routes/redundancy/at-risk.tsx`:
- Around line 15-22: The function formatBytes is duplicated across at-risk.tsx,
compare.tsx, index.tsx, and RedundancyVolumeBar.tsx; extract it into a shared
utility (e.g., create a new helper like formatBytes in a utils/bytes.ts or
similar), export it, and replace the local implementations by importing and
using the shared formatBytes function in each file (ensure the exported function
signature remains formatBytes(bytes: number): string and update imports in the
components that currently define it).
In `@packages/interface/src/routes/redundancy/compare.tsx`:
- Around line 268-289: The ModeButton component uses a template literal to
conditionally set className; replace it with clsx by importing clsx and calling
it in the className prop (keep static classes like "flex-1 rounded-md px-3
py-1.5 text-xs font-medium transition-colors" and apply the conditional object
or array form for active ? "bg-accent text-white" : "text-ink-dull
hover:text-ink"). Update the ModeButton function signature to use
className={clsx(...)} and ensure clsx is added to the module imports.
- Around line 69-83: The search call using useLibraryQuery currently limits
results to 50 (pagination: { limit: 50, offset: 0 }) and is gated by filters,
which hides large result sets; update the component to either implement
pagination state and controls (e.g., page/offset and limit state passed into the
pagination object of useLibraryQuery, showing next/prev or page numbers and
reflecting searchLoading/searchData) or add a "View All" / "Open At-Risk" link
that navigates to the at-risk view with the current filters applied; reference
useLibraryQuery, searchData/searchLoading, filters, NULL_FILTERS and the
pagination object when making the changes so the query updates as users paginate
or follow the link.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 138d65f9-5ed4-401c-8c2b-388c88f973c1
⛔ Files ignored due to path filters (1)
packages/ts-client/src/generated/types.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (18)
core/src/infra/db/migration/m20260414_000001_add_redundancy_indexes.rscore/src/infra/db/migration/mod.rscore/src/ops/mod.rscore/src/ops/redundancy/mod.rscore/src/ops/redundancy/summary/input.rscore/src/ops/redundancy/summary/mod.rscore/src/ops/redundancy/summary/output.rscore/src/ops/redundancy/summary/query.rscore/src/ops/search/filters.rscore/src/ops/search/input.rscore/src/ops/search/mod.rscore/src/ops/search/query.rspackages/interface/src/components/SpacesSidebar/index.tsxpackages/interface/src/router.tsxpackages/interface/src/routes/redundancy/at-risk.tsxpackages/interface/src/routes/redundancy/compare.tsxpackages/interface/src/routes/redundancy/components/RedundancyVolumeBar.tsxpackages/interface/src/routes/redundancy/index.tsx
| // Query 4: Library-wide unique content total (deduplicated) | ||
| let library_total_sql = r#" | ||
| SELECT COALESCE(SUM(ci.total_size), 0) as total_bytes | ||
| FROM content_identities ci | ||
| WHERE ci.id IN ( | ||
| SELECT DISTINCT e.content_id FROM entries e | ||
| WHERE e.content_id IS NOT NULL AND e.volume_id IS NOT NULL | ||
| ) | ||
| "#; | ||
|
|
||
| let library_total = LibraryTotalRow::find_by_statement(Statement::from_string( | ||
| DbBackend::Sqlite, | ||
| library_total_sql.to_string(), | ||
| )) | ||
| .one(db) | ||
| .await?; |
There was a problem hiding this comment.
Inconsistent volume filtering for library totals.
The library_total_sql query does not apply volume_where, so total_unique_content_bytes is always library-wide. However, total_at_risk_bytes and total_redundant_bytes (computed from volume-filtered queries at lines 261-262) respect the volume_uuids filter.
This creates an inconsistency: when a user requests a scoped summary for specific volumes, they get:
total_unique_content_bytes: entire library (unfiltered)total_at_risk_bytes/total_redundant_bytes: filtered to selected volumes
If this is intentional, consider adding a comment clarifying the design choice. Otherwise, apply the volume filter to align all totals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/ops/redundancy/summary/query.rs` around lines 196 - 211, The
library_total_sql query (used to compute total_unique_content_bytes via
LibraryTotalRow::find_by_statement) is missing the volume filter used elsewhere
(volume_where), causing totals to be library-wide while total_at_risk_bytes and
total_redundant_bytes are filtered; update the library_total_sql subquery on
entries (the SELECT DISTINCT e.content_id FROM entries e WHERE ...) to include
the same volume_where condition (or the equivalent e.volume_id IN (...)
predicate) so total_unique_content_bytes respects the requested volume_uuids, or
alternatively add a clear comment next to library_total_sql explaining the
intentional difference if the library-wide behavior is desired.
| .size_range(&self.input.filters.size_range) | ||
| .at_risk(&self.input.filters.at_risk) | ||
| .on_volumes(&self.input.filters.on_volumes) | ||
| .not_on_volumes(&self.input.filters.not_on_volumes) | ||
| .min_volume_count(&self.input.filters.min_volume_count) | ||
| .max_volume_count(&self.input.filters.max_volume_count); |
There was a problem hiding this comment.
Redundancy filters are only applied in the empty-query path.
On Lines 927–932, the new predicates are added only in execute_fast_search_no_fts. execute_fast_search (non-empty FTS flow) and get_total_count still don’t apply these redundancy filters, so search.files behavior and pagination totals diverge by query mode.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/ops/search/query.rs` around lines 927 - 932, The redundancy filters
(size_range, at_risk, on_volumes, not_on_volumes, min_volume_count,
max_volume_count) are only applied in execute_fast_search_no_fts causing
divergence; update execute_fast_search and get_total_count to apply the same
predicates. Best fix: extract the repeated chain into a helper (e.g.,
apply_redundancy_filters) that accepts the query builder and &self.input.filters
and call it from execute_fast_search, execute_fast_search_no_fts, and
get_total_count so both FTS and non-FTS paths and total-count calculations share
identical filter logic.
| export function AtRiskFiles() { | ||
| const navigate = useNavigate(); | ||
| const [searchParams] = useSearchParams(); | ||
| const volumeFilter = searchParams.get("volume"); | ||
| const atRiskParam = searchParams.get("at_risk"); | ||
| const isAtRisk = atRiskParam !== "false"; // default true | ||
|
|
||
| const [offset, setOffset] = useState(0); | ||
|
|
There was a problem hiding this comment.
Reset offset when URL parameters change.
When volumeFilter or isAtRisk changes (via URL search params), the offset state retains its previous value. This can cause the user to see an empty page or stale pagination if they were on page 3, then changed filters resulting in fewer total results.
🐛 Proposed fix using useEffect for reset
import { useState, useMemo } from "react";
+import { useState, useMemo, useEffect } from "react";
import { useSearchParams, useNavigate } from "react-router-dom"; const [offset, setOffset] = useState(0);
+ // Reset pagination when filters change
+ useEffect(() => {
+ setOffset(0);
+ }, [volumeFilter, isAtRisk]);
+
const { data, isLoading } = useLibraryQuery({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function AtRiskFiles() { | |
| const navigate = useNavigate(); | |
| const [searchParams] = useSearchParams(); | |
| const volumeFilter = searchParams.get("volume"); | |
| const atRiskParam = searchParams.get("at_risk"); | |
| const isAtRisk = atRiskParam !== "false"; // default true | |
| const [offset, setOffset] = useState(0); | |
| export function AtRiskFiles() { | |
| const navigate = useNavigate(); | |
| const [searchParams] = useSearchParams(); | |
| const volumeFilter = searchParams.get("volume"); | |
| const atRiskParam = searchParams.get("at_risk"); | |
| const isAtRisk = atRiskParam !== "false"; // default true | |
| // Derive offset from URL or default to 0 | |
| const offset = parseInt(searchParams.get("offset") ?? "0", 10); | |
| const { data, isLoading } = useLibraryQuery({ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/interface/src/routes/redundancy/at-risk.tsx` around lines 26 - 34,
In AtRiskFiles, reset the pagination offset whenever the URL filter parameters
change: add a useEffect in the AtRiskFiles component that calls setOffset(0)
when either volumeFilter or isAtRisk changes so the component does not retain a
stale offset; ensure the effect’s dependency array includes volumeFilter and
isAtRisk (referencing the existing useState offset/setOffset and the
volumeFilter/isAtRisk variables) and avoid adding unrelated dependencies.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # core/src/volume/fs/zfs.rs
- ZFS: override total_capacity for pool-root volumes using zfs list used+available. df under-reports pool-root Size because it only counts the root dataset's own used bytes plus avail — on a 60 TB raidz2 pool this shows as ~15 TB instead of ~62 TB. The pool root's own used property includes descendants, so used+available is the real usable capacity. - Library stats: drop volumes where is_user_visible=false AND re-apply should_hide_by_mount_path retroactively so stale DB rows (detected before the Linux visibility filters existed) don't inflate reported capacity. - Extract should_hide_by_mount_path into volume/utils as a shared helper used by both the list query and the stats calculation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Makes it possible to verify library-level capacity aggregation from the CLI — previously the list only showed mount, fingerprint, and tracked/mounted state, which meant debugging the ZFS pool capacity issue required querying the library DB directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New docs/core/filesystems.mdx covering per-filesystem capabilities (CoW, pool-awareness, visibility filtering, capacity correction), platform detection strategies, the FilesystemHandler trait, Linux/ macOS/ZFS visibility rules, the ZFS pool-root capacity problem and fix, copy strategy selection, and known limitations. Registered under File Management in both mint.json and docs.json. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- core/src/ops/search: wire redundancy filters (at_risk, on_volumes, not_on_volumes, min/max volume_count) through the search query; fix UUID-to-SQLite BLOB literal so volume UUID comparisons actually match (volumes.uuid is stored as a 16-byte BLOB, quoted-string comparison silently returned zero rows). - apps/cli: new redundancy subcommand + populate the new SearchFilters fields from search args. - packages/interface: redundancy at-risk and compare pages reworked to consume the new filter surface; explorer context/hook updates to support redundancy-scoped views. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/interface/src/routes/redundancy/compare.tsx (1)
84-89:⚠️ Potential issue | 🟠 Major
sharedfilter is still overly broad (duplicate of earlier reviewer comment).With
on_volumes: [A, B]treated as "any-of" plusmin_volume_count: 2, a file onA + C(notB) still qualifies as "shared between A & B". Consider either switchingon_volumesto all-of semantics for multi-select or adding a dedicated intersection filter (e.g., reusenot_on_volumesexclusion logic, or add ashared_betweenfilter server-side).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 84 - 89, The "shared" branch currently builds filters using on_volumes + min_volume_count which yields false positives (case "shared", EMPTY_FILTERS, on_volumes, min_volume_count); change the client payload to express an intersection rather than an any-of: replace on_volumes + min_volume_count with a dedicated intersection field (e.g., on_volumes_all or shared_between) set to [volumeA!, volumeB!], and update the server/filter logic to treat that field as "all-of" (or alternatively add a min_count scoped to on_volumes like min_volume_count_on_volumes: 2) so only files present on both A and B are returned.
🧹 Nitpick comments (7)
apps/cli/src/domains/search/args.rs (1)
165-228: Optional: use..Default::default()to future-proof struct init.
SearchFiltersderivesDefault(percore/src/ops/search/input.rs), so explicitly writingat_risk: None, on_volumes: None, …for every non-CLI-backed field risks breaking every time a new filter field is added to the struct. Consider spreading..Default::default()to cover those fields instead.♻️ Proposed refactor
include_hidden: Some(args.include_hidden), include_archived: Some(args.include_archived), - at_risk: None, - on_volumes: None, - not_on_volumes: None, - min_volume_count: None, - max_volume_count: None, + ..Default::default() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/domains/search/args.rs` around lines 165 - 228, The SearchFilters struct is being fully enumerated and manually sets many fields to None, which will break when new fields are added; instead construct SearchFilters using the fields you need (file_types, tags, date_range, size_range, locations, content_types, include_hidden, include_archived) and use ..Default::default() to fill the rest so future additions are covered; update the initializer around SearchFilters (the block that builds tags via TagFilter, date_range via DateRangeFilter, size_range via SizeRangeFilter, and content_types mapping from ContentTypeArg to ContentKind) to only set the required fields and append ..Default::default() at the end.core/src/library/mod.rs (1)
1259-1272: Duplicate visibility predicate across instance and static capacity paths.The same "user-visible +
should_hide_by_mount_path" filter closure is now repeated in bothcalculate_volume_capacityandcalculate_volume_capacity_static. Future tweaks to the predicate (e.g., a newis_system_volume_typecheck) will need to be mirrored in two places or the two paths will silently drift. Consider extracting a small helper likefn is_capacity_eligible(vol: &volume::Model) -> boolso both capacity calculators call the same predicate.♻️ Proposed refactor sketch
fn is_capacity_eligible(vol: &crate::infra::db::entities::volume::Model) -> bool { let volume_type = vol.volume_type.as_deref().unwrap_or("Unknown"); matches!(volume_type, "Primary" | "UserData" | "External" | "Secondary") && vol.is_user_visible.unwrap_or(true) && !vol .mount_point .as_deref() .map(std::path::Path::new) .map(crate::volume::utils::should_hide_by_mount_path) .unwrap_or(false) }Then both call sites become
volumes.into_iter().filter(Self::is_capacity_eligible).collect().Also applies to: 1686-1699
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/library/mod.rs` around lines 1259 - 1272, Extract the duplicated visibility predicate into a single helper function (e.g., fn is_capacity_eligible(vol: &crate::infra::db::entities::volume::Model) -> bool) and have both calculate_volume_capacity and calculate_volume_capacity_static call that helper instead of inlining the closure; implement the helper to combine the existing checks (volume_type inclusion if needed, vol.is_user_visible.unwrap_or(true), and the mount_point -> should_hide_by_mount_path check) and replace the .filter(|vol| { ... }) closures in both calculate_volume_capacity and calculate_volume_capacity_static with .filter(Self::is_capacity_eligible) (or the appropriate path to the helper) so future predicate changes are made in one place.apps/cli/src/domains/volume/mod.rs (1)
110-126: Duplicatedformat_byteshelper — consider extracting to a shared util.
apps/cli/src/main.rs(lines 8–23) already defines a nearly identicalformat_bytesfunction (differs only in decimal precision and inclusion of"PB"). Consider moving a single implementation intoapps/cli/src/util/(e.g.,util/format.rs) and consuming it from both call sites to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/cli/src/domains/volume/mod.rs` around lines 110 - 126, The project contains duplicated format_bytes implementations; extract a single shared helper and update both callers to use it: create a new util module (e.g., util::format or util::format_bytes) that exports a format_bytes function (preserving the desired unit list UNITS and formatting behavior), replace the local format_bytes in apps/cli/src/domains/volume/mod.rs and the other copy in apps/cli/src/main.rs to call the shared util, and update imports/uses accordingly so both call sites use the same implementation to prevent drift.packages/interface/src/routes/redundancy/index.tsx (2)
54-93: DuplicatedTopBarPortalblock between loading and loaded branches.The same portal/title markup is rendered in both branches. Hoist it above the
isLoadingguard so only the body differs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/index.tsx` around lines 54 - 93, Hoist the duplicated TopBarPortal/TopBarItem JSX out of the isLoading guard: move the TopBarPortal block that uses topBarTitle so it renders unconditionally above the existing isLoading || !data check, then simplify the conditional branches (the loading branch and the data-rendering branch in the redundancy component) to only return their respective body content (loading spinner/message or the rendered data) without repeating TopBarPortal; update references to isLoading, data, TopBarPortal, TopBarItem, and topBarTitle accordingly.
32-52: UnnecessaryuseMemocalls.
scorePercentis a trivialMath.round(... * 100)andtopBarTitlehas no dependencies; neither warrants memoization. Per guidelines, reserveuseMemofor genuinely expensive computations.As per coding guidelines: "Use useMemo only when actually needed for expensive computations, not for simple values like string concatenation".
♻️ Proposed cleanup
- const scorePercent = useMemo(() => { - if (!data) return 0; - return Math.round(data.library_totals.replication_score * 100); - }, [data]); + const scorePercent = data + ? Math.round(data.library_totals.replication_score * 100) + : 0; @@ - const topBarTitle = useMemo( - () => ( - <div className="flex items-center gap-2"> - <ShieldCheck size={20} weight="bold" className="text-ink" /> - <h1 className="text-xl font-bold text-ink">Redundancy</h1> - </div> - ), - [], - ); + const topBarTitle = ( + <div className="flex items-center gap-2"> + <ShieldCheck size={20} weight="bold" className="text-ink" /> + <h1 className="text-xl font-bold text-ink">Redundancy</h1> + </div> + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/index.tsx` around lines 32 - 52, Remove unnecessary useMemo wrappers: the scorePercent computation and topBarTitle JSX are trivial and do not need memoization. Replace const scorePercent = useMemo(..., [data]) with a plain computed value using data (e.g., const scorePercent = !data ? 0 : Math.round(data.library_totals.replication_score * 100)), and replace const topBarTitle = useMemo(() => (...), []) with a plain constant JSX assignment (e.g., const topBarTitle = <div.../>). Keep the existing variable names scorePercent and topBarTitle and remove the useMemo import if no longer used.packages/interface/src/routes/redundancy/compare.tsx (2)
106-113: Prefer event-handler driven filtered-mode sync over an effect.Per guidelines, effects shouldn't be used to update parent component state; call
enterFilteredMode/exitFilteredModedirectly from thesetVolumeA/setVolumeB/setModehandlers (you already derivefilterssynchronously). The unmount cleanup at Line 125-127 is the only effect you actually need here.As per coding guidelines: "Never use Effects to update parent component state - call callback in event handler instead for same render pass".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 106 - 113, The useEffect that calls enterFilteredMode/exitFilteredMode based on filters should be removed and those callbacks invoked directly from the event handlers that change filter inputs; update setVolumeA, setVolumeB and setMode handlers to call enterFilteredMode(filters, label) when filters exist and exitFilteredMode() when filters are cleared (ensuring you pass the same `filters`/`label` values you already derive), and keep only the existing unmount cleanup effect (the cleanup that runs on unmount) in the component.
37-43:formatBytesis duplicated across redundancy files.Same helper appears in
compare.tsx,index.tsx, andcomponents/RedundancyVolumeBar.tsx. Extract into a shared util (e.g.,packages/interface/src/routes/redundancy/utils.tsor@sd/uiif broadly useful) to keep units/formatting consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 37 - 43, formatBytes is duplicated; extract the function into a single shared utility and import it where needed. Create a new util module (e.g., redundancy/utils or a shared UI utils) that exports formatBytes, move the logic from the local formatBytes implementations into that exported function, update callers in compare.tsx, index.tsx, and components/RedundancyVolumeBar.tsx to import and use the shared formatBytes, and remove the duplicate local definitions so formatting logic remains consistent across components.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/cli/src/domains/redundancy/mod.rs`:
- Around line 99-105: CompareMode::Shared currently builds a single on_volumes
predicate with min_volume_count=2 which uses OR semantics and can match files
present on A and any other volume (not necessarily B); change the logic for
CompareMode::Shared to require AND semantics by generating two separate
on_volumes predicates (one for args.volume_a and one for args.volume_b) or by
constructing a query that enforces both volumes (e.g., two EXISTS/joins or an
INTERSECT on file UUIDs) instead of relying on min_volume_count; update the code
paths that set on_volumes and min_volume_count for CompareMode::Shared so the
SQL ensures presence on both volumes explicitly rather than OR + count.
In `@core/src/volume/fs/zfs.rs`:
- Around line 434-449: The pool-root override can set volume.total_capacity to 0
when parse_zfs_size fails; compute pool_total =
dataset_info.used_bytes.saturating_add(dataset_info.available_bytes) and only
perform the debug/log and assignments to volume.total_capacity and
volume.available_space when pool_total > 0 (i.e., the sum of
dataset_info.used_bytes and dataset_info.available_bytes is non-zero) in the
same block that currently handles dataset_info.name == dataset_info.pool_name;
this prevents replacing the df-derived capacity with a zero value (see symbols:
dataset_info, pool_total, volume.total_capacity, volume.available_space,
used_bytes, available_bytes, and the parsing site in
find_dataset_for_path/parse_zfs_size).
In `@docs/core/filesystems.mdx`:
- Around line 187-195: Update the descriptive count to match the enumerated
steps in the comment for calculate_volume_capacity (and _static) in
core::library::mod: replace "three passes" with "five steps" (or alternatively
reword to "the following passes" if you prefer not to enumerate) so the prose
and the numbered list are consistent; ensure the symbols
calculate_volume_capacity and _static are unchanged and only the surrounding
documentation text is edited.
In `@packages/interface/src/routes/redundancy/compare.tsx`:
- Around line 253-255: The JSX className string uses the raw Tailwind color
"text-white" when active; replace that with a semantic color token per the
semantic-color rule (for example use "text-ink" or your project's
accent-foreground semantic token) in the ternary branch that reads active ?
"bg-accent text-white" so the active state becomes active ? "bg-accent
<semantic-token>" (locate the className template in compare.tsx where the
variable active is used).
---
Duplicate comments:
In `@packages/interface/src/routes/redundancy/compare.tsx`:
- Around line 84-89: The "shared" branch currently builds filters using
on_volumes + min_volume_count which yields false positives (case "shared",
EMPTY_FILTERS, on_volumes, min_volume_count); change the client payload to
express an intersection rather than an any-of: replace on_volumes +
min_volume_count with a dedicated intersection field (e.g., on_volumes_all or
shared_between) set to [volumeA!, volumeB!], and update the server/filter logic
to treat that field as "all-of" (or alternatively add a min_count scoped to
on_volumes like min_volume_count_on_volumes: 2) so only files present on both A
and B are returned.
---
Nitpick comments:
In `@apps/cli/src/domains/search/args.rs`:
- Around line 165-228: The SearchFilters struct is being fully enumerated and
manually sets many fields to None, which will break when new fields are added;
instead construct SearchFilters using the fields you need (file_types, tags,
date_range, size_range, locations, content_types, include_hidden,
include_archived) and use ..Default::default() to fill the rest so future
additions are covered; update the initializer around SearchFilters (the block
that builds tags via TagFilter, date_range via DateRangeFilter, size_range via
SizeRangeFilter, and content_types mapping from ContentTypeArg to ContentKind)
to only set the required fields and append ..Default::default() at the end.
In `@apps/cli/src/domains/volume/mod.rs`:
- Around line 110-126: The project contains duplicated format_bytes
implementations; extract a single shared helper and update both callers to use
it: create a new util module (e.g., util::format or util::format_bytes) that
exports a format_bytes function (preserving the desired unit list UNITS and
formatting behavior), replace the local format_bytes in
apps/cli/src/domains/volume/mod.rs and the other copy in apps/cli/src/main.rs to
call the shared util, and update imports/uses accordingly so both call sites use
the same implementation to prevent drift.
In `@core/src/library/mod.rs`:
- Around line 1259-1272: Extract the duplicated visibility predicate into a
single helper function (e.g., fn is_capacity_eligible(vol:
&crate::infra::db::entities::volume::Model) -> bool) and have both
calculate_volume_capacity and calculate_volume_capacity_static call that helper
instead of inlining the closure; implement the helper to combine the existing
checks (volume_type inclusion if needed, vol.is_user_visible.unwrap_or(true),
and the mount_point -> should_hide_by_mount_path check) and replace the
.filter(|vol| { ... }) closures in both calculate_volume_capacity and
calculate_volume_capacity_static with .filter(Self::is_capacity_eligible) (or
the appropriate path to the helper) so future predicate changes are made in one
place.
In `@packages/interface/src/routes/redundancy/compare.tsx`:
- Around line 106-113: The useEffect that calls
enterFilteredMode/exitFilteredMode based on filters should be removed and those
callbacks invoked directly from the event handlers that change filter inputs;
update setVolumeA, setVolumeB and setMode handlers to call
enterFilteredMode(filters, label) when filters exist and exitFilteredMode() when
filters are cleared (ensuring you pass the same `filters`/`label` values you
already derive), and keep only the existing unmount cleanup effect (the cleanup
that runs on unmount) in the component.
- Around line 37-43: formatBytes is duplicated; extract the function into a
single shared utility and import it where needed. Create a new util module
(e.g., redundancy/utils or a shared UI utils) that exports formatBytes, move the
logic from the local formatBytes implementations into that exported function,
update callers in compare.tsx, index.tsx, and components/RedundancyVolumeBar.tsx
to import and use the shared formatBytes, and remove the duplicate local
definitions so formatting logic remains consistent across components.
In `@packages/interface/src/routes/redundancy/index.tsx`:
- Around line 54-93: Hoist the duplicated TopBarPortal/TopBarItem JSX out of the
isLoading guard: move the TopBarPortal block that uses topBarTitle so it renders
unconditionally above the existing isLoading || !data check, then simplify the
conditional branches (the loading branch and the data-rendering branch in the
redundancy component) to only return their respective body content (loading
spinner/message or the rendered data) without repeating TopBarPortal; update
references to isLoading, data, TopBarPortal, TopBarItem, and topBarTitle
accordingly.
- Around line 32-52: Remove unnecessary useMemo wrappers: the scorePercent
computation and topBarTitle JSX are trivial and do not need memoization. Replace
const scorePercent = useMemo(..., [data]) with a plain computed value using data
(e.g., const scorePercent = !data ? 0 :
Math.round(data.library_totals.replication_score * 100)), and replace const
topBarTitle = useMemo(() => (...), []) with a plain constant JSX assignment
(e.g., const topBarTitle = <div.../>). Keep the existing variable names
scorePercent and topBarTitle and remove the useMemo import if no longer used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e3be43c-3553-4be0-ac90-bb5d31d3dfef
⛔ Files ignored due to path filters (3)
docs/docs.jsonis excluded by!**/*.jsondocs/mint.jsonis excluded by!**/*.jsonpackages/ts-client/src/generated/types.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (25)
apps/cli/src/domains/mod.rsapps/cli/src/domains/redundancy/args.rsapps/cli/src/domains/redundancy/mod.rsapps/cli/src/domains/search/args.rsapps/cli/src/domains/volume/mod.rsapps/cli/src/main.rscore/src/domain/space.rscore/src/library/manager.rscore/src/library/mod.rscore/src/ops/search/filters.rscore/src/ops/search/query.rscore/src/ops/volumes/list/query.rscore/src/volume/fs/zfs.rscore/src/volume/utils.rsdocs/core/filesystems.mdxpackages/interface/src/components/SpacesSidebar/SpaceCustomizationPanel.tsxpackages/interface/src/components/SpacesSidebar/hooks/spaceItemUtils.tspackages/interface/src/components/SpacesSidebar/index.tsxpackages/interface/src/routes/explorer/ExplorerView.tsxpackages/interface/src/routes/explorer/context.tsxpackages/interface/src/routes/explorer/hooks/useExplorerFiles.tspackages/interface/src/routes/redundancy/at-risk.tsxpackages/interface/src/routes/redundancy/compare.tsxpackages/interface/src/routes/redundancy/components/RedundancyVolumeBar.tsxpackages/interface/src/routes/redundancy/index.tsx
✅ Files skipped from review due to trivial changes (3)
- apps/cli/src/domains/mod.rs
- packages/interface/src/components/SpacesSidebar/index.tsx
- packages/interface/src/routes/redundancy/at-risk.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/interface/src/routes/redundancy/components/RedundancyVolumeBar.tsx
- core/src/ops/search/filters.rs
| CompareMode::Shared => ( | ||
| Some(vec![args.volume_a, args.volume_b]), | ||
| None, | ||
| Some(2u32), | ||
| "shared", | ||
| ), | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm on_volumes uses IN (...) (OR semantics) and there's no AND-of-volumes filter.
rg -nP -C3 'fn on_volumes|fn not_on_volumes|fn min_volume_count' --type=rust
rg -nP -C2 'v\.uuid IN' --type=rustRepository: spacedriveapp/spacedrive
Length of output: 2202
🏁 Script executed:
# Get full on_volumes implementation to see condition chaining
sed -n '135,156p' core/src/ops/search/filters.rsRepository: spacedriveapp/spacedrive
Length of output: 640
🏁 Script executed:
# Check for any AND-based volume filters or on_all_volumes
rg 'on_all_volumes|all_volumes|AND.*uuid|AND.*volume' -C2 --type=rust core/src/ops/search/Repository: spacedriveapp/spacedrive
Length of output: 1849
🏁 Script executed:
# View the full redundancy mod.rs to see all compare modes and filter combinations
sed -n '85,115p' apps/cli/src/domains/redundancy/mod.rsRepository: spacedriveapp/spacedrive
Length of output: 794
🏁 Script executed:
# Check how SearchFilters applies these filters and verify AND chaining
rg -nP 'impl.*SearchFilters|fn apply_filters|apply.*on_volumes.*min_volume' -A5 --type=rust core/src/ops/search/Repository: spacedriveapp/spacedrive
Length of output: 560
🏁 Script executed:
# Look for tests of the compare/shared mode functionality
rg -nP 'test.*shared|test.*compare|SharedContent' -B2 -A8 --type=rustRepository: spacedriveapp/spacedrive
Length of output: 5070
🏁 Script executed:
# Check if there's a struct or method that combines on_volumes
fd -name "*.rs" -type f | xargs grep -l "SearchFilters" | head -5 | xargs -I {} sh -c "echo '===== {} =====' && head -100 {}"Repository: spacedriveapp/spacedrive
Length of output: 298
🏁 Script executed:
# Get the full apply_filters implementation to see how filters chain
sed -n '583,650p' core/src/ops/search/query.rsRepository: spacedriveapp/spacedrive
Length of output: 2578
🏁 Script executed:
# Check if there are any comments in redundancy/mod.rs explaining Shared mode logic
sed -n '75,120p' apps/cli/src/domains/redundancy/mod.rs | head -50Repository: spacedriveapp/spacedrive
Length of output: 1111
Shared mode can return files that are not actually on both volumes.
on_volumes uses OR semantics at the SQL layer (v.uuid IN (...) means "present on any of these volumes"), so on_volumes=[A, B] with min_volume_count=2 will match a file that exists on {A, C} (on A ✓, on 2 volumes ✓) and report it as shared, even though it does not exist on B. To correctly identify content present on both A and B, you need AND semantics—e.g., two separate on_volumes predicates—rather than OR + volume count. As written, the CLI will over-report the shared set whenever a third volume exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/cli/src/domains/redundancy/mod.rs` around lines 99 - 105,
CompareMode::Shared currently builds a single on_volumes predicate with
min_volume_count=2 which uses OR semantics and can match files present on A and
any other volume (not necessarily B); change the logic for CompareMode::Shared
to require AND semantics by generating two separate on_volumes predicates (one
for args.volume_a and one for args.volume_b) or by constructing a query that
enforces both volumes (e.g., two EXISTS/joins or an INTERSECT on file UUIDs)
instead of relying on min_volume_count; update the code paths that set
on_volumes and min_volume_count for CompareMode::Shared so the SQL ensures
presence on both volumes explicitly rather than OR + count.
| if dataset_info.name == dataset_info.pool_name { | ||
| let pool_total = dataset_info | ||
| .used_bytes | ||
| .saturating_add(dataset_info.available_bytes); | ||
| debug!( | ||
| "ZFS pool root '{}' at {}: overriding total_capacity {} → {} (used={}, avail={})", | ||
| dataset_info.pool_name, | ||
| mount_point, | ||
| volume.total_capacity, | ||
| pool_total, | ||
| dataset_info.used_bytes, | ||
| dataset_info.available_bytes, | ||
| ); | ||
| volume.total_capacity = pool_total; | ||
| volume.available_space = dataset_info.available_bytes; | ||
| } |
There was a problem hiding this comment.
Guard against zero pool totals from malformed zfs list parsing.
parse_zfs_size silently falls back to 0 on parse failure (via unwrap_or(0) in find_dataset_for_path). If that happens for a pool-root dataset, this branch will overwrite the df-derived total_capacity with 0, which is strictly worse than the original value. Consider only applying the override when pool_total > 0 (or at least non-zero used_bytes + available_bytes).
🛡️ Proposed fix
if dataset_info.name == dataset_info.pool_name {
let pool_total = dataset_info
.used_bytes
.saturating_add(dataset_info.available_bytes);
- debug!(
- "ZFS pool root '{}' at {}: overriding total_capacity {} → {} (used={}, avail={})",
- dataset_info.pool_name,
- mount_point,
- volume.total_capacity,
- pool_total,
- dataset_info.used_bytes,
- dataset_info.available_bytes,
- );
- volume.total_capacity = pool_total;
- volume.available_space = dataset_info.available_bytes;
+ if pool_total > 0 {
+ debug!(
+ "ZFS pool root '{}' at {}: overriding total_capacity {} → {} (used={}, avail={})",
+ dataset_info.pool_name,
+ mount_point,
+ volume.total_capacity,
+ pool_total,
+ dataset_info.used_bytes,
+ dataset_info.available_bytes,
+ );
+ volume.total_capacity = pool_total;
+ volume.available_space = dataset_info.available_bytes;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if dataset_info.name == dataset_info.pool_name { | |
| let pool_total = dataset_info | |
| .used_bytes | |
| .saturating_add(dataset_info.available_bytes); | |
| debug!( | |
| "ZFS pool root '{}' at {}: overriding total_capacity {} → {} (used={}, avail={})", | |
| dataset_info.pool_name, | |
| mount_point, | |
| volume.total_capacity, | |
| pool_total, | |
| dataset_info.used_bytes, | |
| dataset_info.available_bytes, | |
| ); | |
| volume.total_capacity = pool_total; | |
| volume.available_space = dataset_info.available_bytes; | |
| } | |
| if dataset_info.name == dataset_info.pool_name { | |
| let pool_total = dataset_info | |
| .used_bytes | |
| .saturating_add(dataset_info.available_bytes); | |
| if pool_total > 0 { | |
| debug!( | |
| "ZFS pool root '{}' at {}: overriding total_capacity {} → {} (used={}, avail={})", | |
| dataset_info.pool_name, | |
| mount_point, | |
| volume.total_capacity, | |
| pool_total, | |
| dataset_info.used_bytes, | |
| dataset_info.available_bytes, | |
| ); | |
| volume.total_capacity = pool_total; | |
| volume.available_space = dataset_info.available_bytes; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/volume/fs/zfs.rs` around lines 434 - 449, The pool-root override can
set volume.total_capacity to 0 when parse_zfs_size fails; compute pool_total =
dataset_info.used_bytes.saturating_add(dataset_info.available_bytes) and only
perform the debug/log and assignments to volume.total_capacity and
volume.available_space when pool_total > 0 (i.e., the sum of
dataset_info.used_bytes and dataset_info.available_bytes is non-zero) in the
same block that currently handles dataset_info.name == dataset_info.pool_name;
this prevents replacing the df-derived capacity with a zero value (see symbols:
dataset_info, pool_total, volume.total_capacity, volume.available_space,
used_bytes, available_bytes, and the parsing site in
find_dataset_for_path/parse_zfs_size).
| `calculate_volume_capacity` (and `_static`) in `core/src/library/mod.rs` aggregates per-volume capacity with three passes: | ||
|
|
||
| 1. Filter by `volume_type` (`Primary`, `UserData`, `External`, `Secondary`). | ||
| 2. Filter by visibility (`is_user_visible = true` *and* `!should_hide_by_mount_path(mount)`). | ||
| 3. Deduplicate by fingerprint. | ||
| 4. Sort by mount-path length (shortest first). | ||
| 5. For each volume: skip if it's a subpath of an already-counted volume on the same device; otherwise add its capacity to the running totals. | ||
|
|
||
| Subpath dedup handles the common ZFS case: when `/mnt/pool` is tracked along with `/mnt/pool/footage` and `/mnt/pool/cctv`, only `/mnt/pool` gets counted (once). |
There was a problem hiding this comment.
"three passes" doesn't match the numbered list.
The prose says three passes but the list enumerates five steps. Either update the count (to "five steps") or group/collapse the list.
✏️ Proposed fix
-`calculate_volume_capacity` (and `_static`) in `core/src/library/mod.rs` aggregates per-volume capacity with three passes:
+`calculate_volume_capacity` (and `_static`) in `core/src/library/mod.rs` aggregates per-volume capacity in the following steps:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `calculate_volume_capacity` (and `_static`) in `core/src/library/mod.rs` aggregates per-volume capacity with three passes: | |
| 1. Filter by `volume_type` (`Primary`, `UserData`, `External`, `Secondary`). | |
| 2. Filter by visibility (`is_user_visible = true` *and* `!should_hide_by_mount_path(mount)`). | |
| 3. Deduplicate by fingerprint. | |
| 4. Sort by mount-path length (shortest first). | |
| 5. For each volume: skip if it's a subpath of an already-counted volume on the same device; otherwise add its capacity to the running totals. | |
| Subpath dedup handles the common ZFS case: when `/mnt/pool` is tracked along with `/mnt/pool/footage` and `/mnt/pool/cctv`, only `/mnt/pool` gets counted (once). | |
| `calculate_volume_capacity` (and `_static`) in `core/src/library/mod.rs` aggregates per-volume capacity in the following steps: | |
| 1. Filter by `volume_type` (`Primary`, `UserData`, `External`, `Secondary`). | |
| 2. Filter by visibility (`is_user_visible = true` *and* `!should_hide_by_mount_path(mount)`). | |
| 3. Deduplicate by fingerprint. | |
| 4. Sort by mount-path length (shortest first). | |
| 5. For each volume: skip if it's a subpath of an already-counted volume on the same device; otherwise add its capacity to the running totals. | |
| Subpath dedup handles the common ZFS case: when `/mnt/pool` is tracked along with `/mnt/pool/footage` and `/mnt/pool/cctv`, only `/mnt/pool` gets counted (once). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/core/filesystems.mdx` around lines 187 - 195, Update the descriptive
count to match the enumerated steps in the comment for calculate_volume_capacity
(and _static) in core::library::mod: replace "three passes" with "five steps"
(or alternatively reword to "the following passes" if you prefer not to
enumerate) so the prose and the numbered list are consistent; ensure the symbols
calculate_volume_capacity and _static are unchanged and only the surrounding
documentation text is edited.
| className={`flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors ${ | ||
| active ? "bg-accent text-white" : "text-ink-dull hover:text-ink" | ||
| }`} |
There was a problem hiding this comment.
Use a semantic color instead of text-white.
Per the semantic-color rule, avoid raw Tailwind palette classes. Swap text-white for a semantic token (e.g., text-ink or an accent-paired ink variant).
As per coding guidelines: "Use semantic Tailwind color classes only - never use arbitrary gray values... use bg-app or bg-sidebar instead".
🎨 Proposed fix
- active ? "bg-accent text-white" : "text-ink-dull hover:text-ink"
+ active ? "bg-accent text-ink" : "text-ink-dull hover:text-ink"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={`flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors ${ | |
| active ? "bg-accent text-white" : "text-ink-dull hover:text-ink" | |
| }`} | |
| className={`flex-1 rounded-md px-3 py-1.5 text-xs font-medium transition-colors ${ | |
| active ? "bg-accent text-ink" : "text-ink-dull hover:text-ink" | |
| }`} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/interface/src/routes/redundancy/compare.tsx` around lines 253 - 255,
The JSX className string uses the raw Tailwind color "text-white" when active;
replace that with a semantic color token per the semantic-color rule (for
example use "text-ink" or your project's accent-foreground semantic token) in
the ternary branch that reads active ? "bg-accent text-white" so the active
state becomes active ? "bg-accent <semantic-token>" (locate the className
template in compare.tsx where the variable active is used).
- New WebContextMenuProvider + Radix DropdownMenu-based renderer anchored at cursor via a 1x1 virtual trigger. Handles separators, submenus, disabled, and the danger variant via text-status-error. - useContextMenu now routes web clicks through the provider instead of parking data in unused local state, and trims leading/trailing/adjacent separators so condition-filtered menus don't render orphaned lines. - Drop app-frame corner rounding on the web build. - Add shrink-0 to the sidebar space switcher so the scrollable sibling can't compress it vertically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…a dir - build.rs runs `bun run build` in apps/web so `just dev-server` always embeds the latest UI. rerun-if-changed covers apps/web/src, packages/interface/src, and packages/ts-client/src so Rust-only edits skip the rebuild. Skips gracefully when bun isn't on PATH or SD_SKIP_WEB_BUILD is set; Dockerfile sets the latter since dist is pre-built and bun isn't in the Rust stage. - Graceful shutdown was hanging because the browser holds the /events SSE stream open forever and axum waits for all connections to drain. After the first signal, arm a background force-exit on second Ctrl+C or 5s timeout so the process can't stick. - Debug builds were starting from a fresh tempfile::tempdir() on every run (the TempDir handle dropped at end of the closure, deleting the dir we just took a path to). Default to ~/.spacedrive in debug so data persists and `just dev-server` shares a data dir with the Tauri app. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Uses zig cc as C/C++ compiler on TrueNAS Scale where /usr is read-only and no system gcc exists. Dev tools live at /mnt/pool/dev-tools/ (zig, cmake, make, extracted deb headers). Builds sd-server + sd-cli in ~4 min on a 12-core NAS. AI feature disabled (whisper.cpp C11 atomics incompatible with zig clang-18). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/interface/src/contexts/WebContextMenuContext.tsx (2)
33-41:show()adds a frame of latency on the cold path.The
setState(null)→requestAnimationFrame(…setState(new))dance is only needed to force Radix to re-anchor when a menu is already open. On the common cold-open path (state is alreadynull), this still delays the menu by one animation frame and can feel laggy on right-click. Consider branching: open immediately whenstate === null, and only null-then-rAF when replacing an existing menu.♻️ Proposed fix
const show = useCallback( (items: ContextMenuItem[], x: number, y: number) => { - // If a menu is already open, close it first so Radix re-anchors - // at the new cursor position on the next tick. - setState(null); - requestAnimationFrame(() => setState({ items, x, y })); + setState((prev) => { + if (prev === null) return { items, x, y }; + // A menu is already open; close it so Radix re-anchors at + // the new cursor position on the next frame. + requestAnimationFrame(() => setState({ items, x, y })); + return null; + }); }, [], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/contexts/WebContextMenuContext.tsx` around lines 33 - 41, The show() callback adds an unnecessary animation-frame delay on cold opens; change its logic in WebContextMenuContext's show function to check the current state and only perform the null-then-requestAnimationFrame dance when replacing an open menu—i.e., if state === null call setState({ items, x, y }) immediately, otherwise call setState(null) and requestAnimationFrame(() => setState({ items, x, y })); keep the same parameters and use the existing setState and state identifiers.
67-79: Prefer Tailwind for the static trigger styles; keep inline only for dynamic coords.Per the project guideline against inline
styletags, the fixed positioning, 1×1 box, andpointer-events: noneon the trigger can all move to utility classes; onlyleft/toptruly need to be inline because they're dynamic.♻️ Proposed fix
- <DropdownMenu.Trigger asChild> - <span - aria-hidden - style={{ - position: "fixed", - left: state?.x ?? 0, - top: state?.y ?? 0, - width: 1, - height: 1, - pointerEvents: "none", - }} - /> - </DropdownMenu.Trigger> + <DropdownMenu.Trigger asChild> + <span + aria-hidden + className="pointer-events-none fixed size-px" + style={{ left: state?.x ?? 0, top: state?.y ?? 0 }} + /> + </DropdownMenu.Trigger>As per coding guidelines: "Never use
<style>,<style jsx>, or inline style tags - always use Tailwind utility classes instead".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/contexts/WebContextMenuContext.tsx` around lines 67 - 79, The span used as the DropdownMenu.Trigger should move all static CSS into Tailwind classes and keep only the dynamic left/top inline: replace the inline style block on the span with a className like "fixed w-px h-px pointer-events-none" (or "w-[1px] h-[1px]" if you prefer) and leave style={{ left: state?.x ?? 0, top: state?.y ?? 0 }} to set coordinates; keep aria-hidden and the surrounding DropdownMenu.Trigger unchanged so only the static positioning/size/pointer-events are migrated to Tailwind.packages/interface/src/hooks/useContextMenu.ts (1)
59-76:collapseSeparatorslogic looks solid.Leading/trailing trimming and run collapsing are handled correctly, and submenu recursion matches what
WebContextMenu.renderItemsexpects. One small note: if a submenu item'ssubmenucollapses to[]after filtering,renderItemswill fall through to rendering it as a normalItem(itslength > 0check), which will make it look clickable even though it was meant to be a parent. Probably fine for now, but worth dropping such items if you see it in practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/interface/src/hooks/useContextMenu.ts` around lines 59 - 76, collapseSeparators can return an empty submenu which later causes WebContextMenu.renderItems to render the parent as a clickable Item; update collapseSeparators so that when handling an item with item.submenu you compute const collapsed = collapseSeparators(item.submenu) and only push { ...item, submenu: collapsed } if collapsed.length > 0, otherwise skip that parent item entirely; reference collapseSeparators and WebContextMenu.renderItems to locate the behavior.build-server.sh (2)
6-6: Preferset -euo pipefail.
set -ealone won't catch unset variables ($SRtypos) or failures inside pipelines. For a build script that exports a lot of environment variables and then shells out to Cargo,set -euo pipefailis the safer default.♻️ Proposed fix
-set -e +set -euo pipefail🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-server.sh` at line 6, Replace the current bash safety setting "set -e" in build-server.sh with the stronger "set -euo pipefail" to fail on unset variables and pipeline errors; update the top-level shell invocation or the block where "set -e" appears (search for the literal set -e) to use set -euo pipefail so the script aborts on unset vars (-u) and pipeline failures (-o pipefail).
1-24: Is this environment-specific script intended to land in the repo?Every path in this script is hardcoded to a single host (
/mnt/pool/dev-tools,/mnt/pool/spacedrive,/mnt/pool/dev-tools/sysroot,-j10). It's clearly useful on one specific TrueNAS box, but as committed it won't work for any other contributor and will rot silently when those paths change.A few options worth considering:
- Move it under
scripts/(or similar) and parameterize the paths with env vars (SPACEDRIVE_SYSROOT,SPACEDRIVE_SRC) with sensible defaults, so the script is reusable.- Keep it out of the repo and place it in dotfiles/ops tooling on the host.
- At minimum, rename to something scoped (
build-server-truenas.sh) and document the assumption in the header.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-server.sh` around lines 1 - 24, The script build-server.sh contains host-specific hardcoded paths and flags (e.g. SR=/mnt/pool/dev-tools/sysroot, PATH entries, CC/CXX/AR exports, OPENSSL_* dirs, cd /mnt/pool/spacedrive and the -j10 in the cargo build invocation) that will break for other contributors; make the script reusable by parameterizing these values via environment variables with sensible defaults (e.g. SPACEDRIVE_SYSROOT, SPACEDRIVE_DEVTOOLS, SPACEDRIVE_SRC, BUILD_JOBS) and update uses of SR, PATH, CC/CXX/AR, OPENSSL_INCLUDE_DIR/OPENSSL_LIB_DIR, and the cargo build -j invocation to reference those variables; additionally either move the file to scripts/ or rename it to reflect its TrueNAS scope (e.g. build-server-truenas.sh) and add a header comment documenting the assumptions if you keep it in the repo.apps/server/build.rs (1)
9-12: Consider validatingapps/web/distwhen the web build is skipped.Both the
SD_SKIP_WEB_BUILDbranch and the "bun not found" branch exit without checking thatapps/web/dist/(or at leastdist/index.html) actually exists. If it doesn't,rust_embed's#[folder = "../web/dist/"]onWebAssetswill still compile — it just yields an empty asset set — and the server will happily start and return the "Spacedrive web UI is not bundled in this build" 404 fromserve_webfor every request.A quick
repo_root.join("apps/web/dist/index.html").exists()guard emittingcargo:warning=(or panicking) would turn that silent misconfiguration into a build-time signal — especially valuable for the Docker stage where forgetting theCOPY apps/web/dist ./apps/web/diststep would otherwise only be caught at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/build.rs` around lines 9 - 12, When skipping the web build (env::var_os("SD_SKIP_WEB_BUILD").is_some()) or when the "bun not found" branch returns early, add a filesystem check using repo_root.join("apps/web/dist/index.html").exists() (or at least repo_root.join("apps/web/dist").exists()) and emit a clear build-time signal (println!("cargo:warning=...") or panic!) if the file/dir is missing so rust_embed's WebAssets (#[folder = "../web/dist/"]) doesn't silently compile with empty assets and serve_web later returns the 404; update both the SD_SKIP_WEB_BUILD branch and the bun-not-found early-exit path to perform this validation and log/panic accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/build.rs`:
- Around line 16-19: The bun availability check currently only treats process
spawn errors as missing; change the check that calls
Command::new("bun").arg("--version").output() so it handles both Err and
non-successful ExitStatus: capture the Result from output(), and if it is Err OR
the returned output.status.success() is false, log the same cargo:warning and
return (so a non-zero exit or broken bun falls back to apps/web/dist). Update
the code around Command::new("bun").arg("--version").output() to perform this
combined check and include the output or status in the warning if you want more
context.
In `@apps/server/src/main.rs`:
- Around line 547-560: The forced-exit spawned task currently calls
std::process::exit(0) for both the second Ctrl+C and the 5s timeout, which
should instead signal abnormal termination; update the tokio::spawn block (the
code using tokio::spawn, signal::ctrl_c, tokio::time::sleep and
Duration::from_secs) so that the branches that represent abnormal termination
(the second signal branch and the timeout branch) call std::process::exit with a
non-zero status (e.g. exit(1)), leaving normal graceful shutdown behavior
unchanged.
- Around line 315-319: The closure that creates the fallback temp dir drops the
TempDir guard (temp) and returns only a PathBuf, leaving base_data_dir pointing
at a deleted directory; fix by converting the TempDir into a persistent path
before returning so the guard isn't dropped — e.g., inside the unwrap_or_else
closure call TempDir::into_path() (or TempDir::keep()/persist() if your tempfile
version uses that) on temp and return that resulting PathBuf; update the closure
that binds temp (the TempDir) so it is consumed into a persistent path and
handle any errors from persist/keep accordingly so base_data_dir remains valid
for subsequent file operations.
In `@build-server.sh`:
- Around line 2-4: The header comment incorrectly claims "Uses zig cc as C
compiler" while the script exports CC and CXX as /mnt/pool/dev-tools/cc and
/mnt/pool/dev-tools/c++; update the header to accurately describe the actual
compiler setup: either state that CC and CXX point to local wrapper scripts that
invoke zig cc (if they are wrappers) or remove/replace the zig mention and say
the script uses the compilers at CC=/mnt/pool/dev-tools/cc and
CXX=/mnt/pool/dev-tools/c++; reference the exported variables CC and CXX in the
revised comment to make the relationship clear.
In `@packages/interface/src/hooks/useContextMenu.ts`:
- Line 147: Remove the unnecessary any cast when reading __SPACEDRIVE__ in
useContextMenu.ts: replace the line that assigns nativeShow from (window as
any).__SPACEDRIVE__?.showContextMenu with a plain
window.__SPACEDRIVE__?.showContextMenu (the Window interface in env.d.ts already
declares __SPACEDRIVE__), so nativeShow uses the typed window without using any.
---
Nitpick comments:
In `@apps/server/build.rs`:
- Around line 9-12: When skipping the web build
(env::var_os("SD_SKIP_WEB_BUILD").is_some()) or when the "bun not found" branch
returns early, add a filesystem check using
repo_root.join("apps/web/dist/index.html").exists() (or at least
repo_root.join("apps/web/dist").exists()) and emit a clear build-time signal
(println!("cargo:warning=...") or panic!) if the file/dir is missing so
rust_embed's WebAssets (#[folder = "../web/dist/"]) doesn't silently compile
with empty assets and serve_web later returns the 404; update both the
SD_SKIP_WEB_BUILD branch and the bun-not-found early-exit path to perform this
validation and log/panic accordingly.
In `@build-server.sh`:
- Line 6: Replace the current bash safety setting "set -e" in build-server.sh
with the stronger "set -euo pipefail" to fail on unset variables and pipeline
errors; update the top-level shell invocation or the block where "set -e"
appears (search for the literal set -e) to use set -euo pipefail so the script
aborts on unset vars (-u) and pipeline failures (-o pipefail).
- Around line 1-24: The script build-server.sh contains host-specific hardcoded
paths and flags (e.g. SR=/mnt/pool/dev-tools/sysroot, PATH entries, CC/CXX/AR
exports, OPENSSL_* dirs, cd /mnt/pool/spacedrive and the -j10 in the cargo build
invocation) that will break for other contributors; make the script reusable by
parameterizing these values via environment variables with sensible defaults
(e.g. SPACEDRIVE_SYSROOT, SPACEDRIVE_DEVTOOLS, SPACEDRIVE_SRC, BUILD_JOBS) and
update uses of SR, PATH, CC/CXX/AR, OPENSSL_INCLUDE_DIR/OPENSSL_LIB_DIR, and the
cargo build -j invocation to reference those variables; additionally either move
the file to scripts/ or rename it to reflect its TrueNAS scope (e.g.
build-server-truenas.sh) and add a header comment documenting the assumptions if
you keep it in the repo.
In `@packages/interface/src/contexts/WebContextMenuContext.tsx`:
- Around line 33-41: The show() callback adds an unnecessary animation-frame
delay on cold opens; change its logic in WebContextMenuContext's show function
to check the current state and only perform the null-then-requestAnimationFrame
dance when replacing an open menu—i.e., if state === null call setState({ items,
x, y }) immediately, otherwise call setState(null) and requestAnimationFrame(()
=> setState({ items, x, y })); keep the same parameters and use the existing
setState and state identifiers.
- Around line 67-79: The span used as the DropdownMenu.Trigger should move all
static CSS into Tailwind classes and keep only the dynamic left/top inline:
replace the inline style block on the span with a className like "fixed w-px
h-px pointer-events-none" (or "w-[1px] h-[1px]" if you prefer) and leave
style={{ left: state?.x ?? 0, top: state?.y ?? 0 }} to set coordinates; keep
aria-hidden and the surrounding DropdownMenu.Trigger unchanged so only the
static positioning/size/pointer-events are migrated to Tailwind.
In `@packages/interface/src/hooks/useContextMenu.ts`:
- Around line 59-76: collapseSeparators can return an empty submenu which later
causes WebContextMenu.renderItems to render the parent as a clickable Item;
update collapseSeparators so that when handling an item with item.submenu you
compute const collapsed = collapseSeparators(item.submenu) and only push {
...item, submenu: collapsed } if collapsed.length > 0, otherwise skip that
parent item entirely; reference collapseSeparators and
WebContextMenu.renderItems to locate the behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f8ab7e56-fadc-46aa-bfea-0e30f522fad6
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lock,!**/*.lockapps/server/Cargo.tomlis excluded by!**/*.toml
📒 Files selected for processing (9)
apps/server/Dockerfileapps/server/build.rsapps/server/src/main.rsbuild-server.shcore/src/library/manager.rspackages/interface/src/ShellLayout.tsxpackages/interface/src/components/SpacesSidebar/SpaceSwitcher.tsxpackages/interface/src/contexts/WebContextMenuContext.tsxpackages/interface/src/hooks/useContextMenu.ts
✅ Files skipped from review due to trivial changes (1)
- packages/interface/src/components/SpacesSidebar/SpaceSwitcher.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/library/manager.rs
| if Command::new("bun").arg("--version").output().is_err() { | ||
| println!("cargo:warning=bun not found on PATH — using existing apps/web/dist"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
bun detection only catches spawn failures.
Command::new("bun").arg("--version").output().is_err() returns Err only if the OS failed to spawn the process (typically "not found on PATH"). If a bun executable is present but broken (shim that exits non-zero, wrong arch, permission denied after spawn, etc.), output() is Ok with an unsuccessful ExitStatus, the check passes, and the script will go on to call bun run build and panic later instead of falling back to the prebuilt dist. Consider also inspecting status.success() before treating bun as usable.
♻️ Proposed fix
- if Command::new("bun").arg("--version").output().is_err() {
- println!("cargo:warning=bun not found on PATH — using existing apps/web/dist");
- return;
- }
+ let bun_ok = Command::new("bun")
+ .arg("--version")
+ .output()
+ .map(|o| o.status.success())
+ .unwrap_or(false);
+ if !bun_ok {
+ println!("cargo:warning=bun not found or not runnable on PATH — using existing apps/web/dist");
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if Command::new("bun").arg("--version").output().is_err() { | |
| println!("cargo:warning=bun not found on PATH — using existing apps/web/dist"); | |
| return; | |
| } | |
| let bun_ok = Command::new("bun") | |
| .arg("--version") | |
| .output() | |
| .map(|o| o.status.success()) | |
| .unwrap_or(false); | |
| if !bun_ok { | |
| println!("cargo:warning=bun not found or not runnable on PATH — using existing apps/web/dist"); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/build.rs` around lines 16 - 19, The bun availability check
currently only treats process spawn errors as missing; change the check that
calls Command::new("bun").arg("--version").output() so it handles both Err and
non-successful ExitStatus: capture the Result from output(), and if it is Err OR
the returned output.status.success() is false, log the same cargo:warning and
return (so a non-zero exit or broken bun falls back to apps/web/dist). Update
the code around Command::new("bun").arg("--version").output() to perform this
combined check and include the output or status in the warning if you want more
context.
| .unwrap_or_else(|_| { | ||
| warn!("Could not resolve home directory; falling back to tempdir"); | ||
| let temp = tempfile::tempdir().expect("Failed to create temp dir"); | ||
| temp.path().to_path_buf() | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
tempfile crate TempDir keep vs into_path API current method name
💡 Result:
In the current version of the tempfile crate (3.27.0 as of 2026-03-11), the TempDir struct provides methods to control automatic deletion of the temporary directory. - TempDir::keep(self) -> PathBuf: Persists the temporary directory to disk, consuming the TempDir and returning its PathBuf. The directory is no longer automatically deleted. - TempDir::into_path(self) -> PathBuf: Deprecated alias for TempDir::keep. Marked as deprecated in recent docs, use keep instead. - TempDir::disable_cleanup(&mut self, disable_cleanup: bool): Disables automatic cleanup in-place without consuming the TempDir. Primarily for testing/debugging; prefer keep to turn it into a non-temporary directory. Example usage: use tempfile::TempDir; use std::fs; let dir = TempDir::new?; // Use dir... let path = dir.keep; // Directory persists at path // or dir.into_path; // Deprecated, same effect keep vs into_path: keep is the current recommended method name; into_path is deprecated but still functional as an alias. Official documentation: https://docs.rs/tempfile/latest/tempfile/struct.TempDir.html (latest version confirmed via crates.io and docs.rs redirects).
Citations:
- 1: https://docs.rs/tempfile/latest/tempfile/dir/struct.TempDir.html
- 2: https://docs.rs/tempfile/
- 3: https://docs.rs/tempfile/latest/tempfile/index.html
- 4: https://crates.io/crates/tempfile
- 5: https://shadow.github.io/docs/rust/tempfile/struct.TempDir.html
- 6: https://docs.rs/tempfile/3.3.0/tempfile/struct.TempDir.html
- 7: https://docs.rs/tempfile/latest/tempfile/struct.TempDir.html
🏁 Script executed:
find . -name "main.rs" -path "*/apps/server/src/*" | head -5Repository: spacedriveapp/spacedrive
Length of output: 93
🏁 Script executed:
# Find the main.rs file in apps/server
fd -t f "main.rs" | grep "apps/server/src"Repository: spacedriveapp/spacedrive
Length of output: 91
🏁 Script executed:
sed -n '310,325p' apps/server/src/main.rsRepository: spacedriveapp/spacedrive
Length of output: 574
🏁 Script executed:
sed -n '300,330p' apps/server/src/main.rsRepository: spacedriveapp/spacedrive
Length of output: 1151
🏁 Script executed:
sed -n '280,340p' apps/server/src/main.rsRepository: spacedriveapp/spacedrive
Length of output: 1956
🏁 Script executed:
# Verify what happens with base_data_dir after it's assigned
sed -n '325,360p' apps/server/src/main.rsRepository: spacedriveapp/spacedrive
Length of output: 1286
Dev tempdir fallback is destroyed immediately.
tempfile::tempdir() returns a TempDir guard whose Drop impl deletes the directory. Binding it to temp and returning only temp.path().to_path_buf() drops the guard at the end of the closure, leaving base_data_dir holding a PathBuf to a directory that has already been removed. Any subsequent file operations (e.g., base_data_dir.join("instances")) will fail or produce undefined behavior.
Use TempDir::keep() to detach the directory from the guard and persist it:
Proposed fix
.unwrap_or_else(|_| {
warn!("Could not resolve home directory; falling back to tempdir");
- let temp = tempfile::tempdir().expect("Failed to create temp dir");
- temp.path().to_path_buf()
+ tempfile::tempdir()
+ .expect("Failed to create temp dir")
+ .keep()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .unwrap_or_else(|_| { | |
| warn!("Could not resolve home directory; falling back to tempdir"); | |
| let temp = tempfile::tempdir().expect("Failed to create temp dir"); | |
| temp.path().to_path_buf() | |
| }) | |
| .unwrap_or_else(|_| { | |
| warn!("Could not resolve home directory; falling back to tempdir"); | |
| tempfile::tempdir() | |
| .expect("Failed to create temp dir") | |
| .keep() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/main.rs` around lines 315 - 319, The closure that creates the
fallback temp dir drops the TempDir guard (temp) and returns only a PathBuf,
leaving base_data_dir pointing at a deleted directory; fix by converting the
TempDir into a persistent path before returning so the guard isn't dropped —
e.g., inside the unwrap_or_else closure call TempDir::into_path() (or
TempDir::keep()/persist() if your tempfile version uses that) on temp and return
that resulting PathBuf; update the closure that binds temp (the TempDir) so it
is consumed into a persistent path and handle any errors from persist/keep
accordingly so base_data_dir remains valid for subsequent file operations.
|
|
||
| // Force-exit fallback: if graceful shutdown doesn't finish (e.g. a browser | ||
| // is holding an SSE connection open), exit on the next Ctrl+C or after 5s. | ||
| tokio::spawn(async { | ||
| tokio::select! { | ||
| _ = signal::ctrl_c() => { | ||
| warn!("Second signal received, forcing exit"); | ||
| } | ||
| _ = tokio::time::sleep(Duration::from_secs(5)) => { | ||
| warn!("Graceful shutdown timed out after 5s, forcing exit"); | ||
| } | ||
| } | ||
| std::process::exit(0); | ||
| }); |
There was a problem hiding this comment.
Force-exit should signal a non-zero exit code on timeout.
A forced exit caused by a 5-second timeout (or a second signal after a first one was already handled) is an abnormal termination — downstream supervisors (systemd, Docker, Kubernetes, CI) typically rely on a non-zero exit status to distinguish "clean graceful shutdown" from "we had to kill ourselves because a connection was stuck". Exiting with 0 in both branches hides that signal and can mask bugs where graceful shutdown is routinely exceeding the budget.
Also worth noting: once axum's graceful shutdown completes normally, main returns and the runtime is dropped, which cancels this spawned task — so the behavior only fires when graceful shutdown is actually stuck, which is exactly the case you want to flag as non-zero.
♻️ Proposed fix
tokio::spawn(async {
tokio::select! {
_ = signal::ctrl_c() => {
warn!("Second signal received, forcing exit");
}
_ = tokio::time::sleep(Duration::from_secs(5)) => {
warn!("Graceful shutdown timed out after 5s, forcing exit");
}
}
- std::process::exit(0);
+ std::process::exit(1);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/server/src/main.rs` around lines 547 - 560, The forced-exit spawned task
currently calls std::process::exit(0) for both the second Ctrl+C and the 5s
timeout, which should instead signal abnormal termination; update the
tokio::spawn block (the code using tokio::spawn, signal::ctrl_c,
tokio::time::sleep and Duration::from_secs) so that the branches that represent
abnormal termination (the second signal branch and the timeout branch) call
std::process::exit with a non-zero status (e.g. exit(1)), leaving normal
graceful shutdown behavior unchanged.
| # Build sd-server + sd-cli natively on TrueNAS Scale | ||
| # Uses zig cc as C compiler (since gcc/clang not installed) | ||
| # Dev tools at /mnt/pool/dev-tools/ |
There was a problem hiding this comment.
Header comment disagrees with the exported compiler.
The header says "Uses zig cc as C compiler", but CC=/mnt/pool/dev-tools/cc and CXX=/mnt/pool/dev-tools/c++ point at plain binaries, not zig cc. If those are wrapper scripts that shell out to zig cc, say so; otherwise update the comment so future readers don't chase a zig toolchain that isn't actually in use.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build-server.sh` around lines 2 - 4, The header comment incorrectly claims
"Uses zig cc as C compiler" while the script exports CC and CXX as
/mnt/pool/dev-tools/cc and /mnt/pool/dev-tools/c++; update the header to
accurately describe the actual compiler setup: either state that CC and CXX
point to local wrapper scripts that invoke zig cc (if they are wrappers) or
remove/replace the zig mention and say the script uses the compilers at
CC=/mnt/pool/dev-tools/cc and CXX=/mnt/pool/dev-tools/c++; reference the
exported variables CC and CXX in the revised comment to make the relationship
clear.
| if (isTauri) { | ||
| // Native mode: Use Tauri's native menu API | ||
| console.log('[useContextMenu] Using Tauri native menu'); | ||
| const nativeShow = (window as any).__SPACEDRIVE__?.showContextMenu; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm __SPACEDRIVE__ is declared on the Window interface so the cast is unnecessary.
fd -t f 'env.d.ts' | xargs rg -n -C3 '__SPACEDRIVE__|showContextMenu'
rg -nP '\binterface\s+Window\b' -C5Repository: spacedriveapp/spacedrive
Length of output: 1002
🏁 Script executed:
cat -n packages/interface/src/hooks/useContextMenu.ts | sed -n '140,155p'Repository: spacedriveapp/spacedrive
Length of output: 717
Remove unnecessary any cast on window.
Line 147 uses (window as any) to access __SPACEDRIVE__, but this type is already declared on the Window interface via apps/tauri/src/env.d.ts. The cast violates the project rule against using any types.
Fix
- const nativeShow = (window as any).__SPACEDRIVE__?.showContextMenu;
+ const nativeShow = window.__SPACEDRIVE__?.showContextMenu;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const nativeShow = (window as any).__SPACEDRIVE__?.showContextMenu; | |
| const nativeShow = window.__SPACEDRIVE__?.showContextMenu; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/interface/src/hooks/useContextMenu.ts` at line 147, Remove the
unnecessary any cast when reading __SPACEDRIVE__ in useContextMenu.ts: replace
the line that assigns nativeShow from (window as
any).__SPACEDRIVE__?.showContextMenu with a plain
window.__SPACEDRIVE__?.showContextMenu (the Window interface in env.d.ts already
declares __SPACEDRIVE__), so nativeShow uses the typed window without using any.
On first load (fresh library, all stats zero), libraries.info used to calculate statistics synchronously before responding. On large libraries during active indexing this hangs indefinitely — the closure-table walk in calculate_file_statistics loads every descendant ID into a Vec then issues a WHERE IN(...) with millions of entries, which SQLite can't finish while the indexer is writing. Now always return cached (possibly zero) stats and let the background recalculate_statistics task fill them in. The UI refreshes via the ResourceChanged event when the calculation completes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/src/ops/libraries/info/query.rs (1)
59-64:⚠️ Potential issue | 🟡 Minor
is_staleconflates "never calculated" with "genuinely empty library."A library that's legitimately empty (no files, no locations, no tags) will match
is_staleon everylibraries.infocall, causing a backgroundrecalculate_statisticsto be spawned every time the query runs — even though the last computation was accurate. Consider using an explicit sentinel (e.g.Option<Statistics>in config, or alast_calculated_attimestamp) so you can distinguish "never populated" from "zero". Not a correctness bug, but it defeats some of the caching benefit this PR is introducing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/libraries/info/query.rs` around lines 59 - 64, The current check around cached_stats (config.statistics) using is_stale conflates a legitimately empty library with "never calculated" because it only tests zero counts; change config.statistics to carry an explicit sentinel (e.g. Option<Statistics> or add a last_calculated_at/timestamp) and update the libraries.info logic that currently computes is_stale to instead check that sentinel (e.g. statistics.is_none() or timestamp is missing/too old) before spawning recalculate_statistics; update references to cached_stats, is_stale, config.statistics and the recalculate_statistics call so only genuinely uncomputed or expired stats trigger background recomputation.
🧹 Nitpick comments (3)
core/src/ops/libraries/info/query.rs (3)
11-11: Nit:use tracing;is redundant.
tracingis already in scope as an extern crate; the bareuse tracing;statement doesn't add anything since you call the macros via fully-qualified paths (tracing::info!, etc.). Either drop this line or switch touse tracing::{debug, info, warn};and call the macros unqualified for brevity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/libraries/info/query.rs` at line 11, The `use tracing;` import is redundant because macros are already invoked via fully-qualified paths (e.g., tracing::info!); remove the bare `use tracing;` line or replace it with a selective import like `use tracing::{debug, info, warn};` and then call `info!`, `debug!`, `warn!` unqualified in functions such as those that currently call `tracing::info!`, ensuring imports match the macros actually used (update any macro calls if you switch to unqualified names).
65-109: Deduplicate therecalculate_statisticstrigger across both branches.Both the stale and non-stale branches call
library.recalculate_statistics().awaitwith near-identical warn-on-error plumbing and both returncached_stats. The only real divergence is the initial log message. You can collapse this to a single trigger after theif is_stalelog, which also makes the intent clearer: always return what's cached, always kick off a background recalc.♻️ Proposed simplification
- let statistics = if is_stale { - // First load or completely empty — return zeros immediately and - // calculate in the background. The synchronous path used to - // block here, but on large libraries (e.g. NAS with millions of - // files being indexed) the closure-table walk in - // calculate_file_statistics can take minutes, locking up the RPC - // endpoint and making the UI unresponsive. The background task - // emits a ResourceChanged event when done so the UI refreshes. - tracing::info!( - library_id = %library_id, - library_name = %config.name, - "Cached statistics are empty, returning zeros and calculating in background" - ); - - if let Err(e) = library.recalculate_statistics().await { - tracing::warn!( - library_id = %library_id, - library_name = %config.name, - error = %e, - "Failed to trigger background statistics calculation" - ); - } - - cached_stats - } else { - // Return cached statistics immediately (non-blocking) - tracing::debug!( - library_id = %library_id, - library_name = %config.name, - "Returning cached statistics and triggering background recalculation" - ); - - // Trigger background recalculation (non-blocking) - // This will emit a ResourceChanged event when complete - if let Err(e) = library.recalculate_statistics().await { - tracing::warn!( - library_id = %library_id, - library_name = %config.name, - error = %e, - "Failed to trigger background statistics recalculation" - ); - } - - cached_stats - }; + // Large libraries (e.g. NAS with millions of indexed files) make the closure-table + // walk in calculate_file_statistics take minutes, so we never block the RPC here — + // the background task emits a ResourceChanged event to refresh the UI. + if is_stale { + tracing::info!( + library_id = %library_id, + library_name = %config.name, + "Cached statistics are empty, returning zeros and calculating in background" + ); + } else { + tracing::debug!( + library_id = %library_id, + library_name = %config.name, + "Returning cached statistics and triggering background recalculation" + ); + } + + if let Err(e) = library.recalculate_statistics().await { + tracing::warn!( + library_id = %library_id, + library_name = %config.name, + error = %e, + "Failed to trigger background statistics recalculation" + ); + } + + let statistics = cached_stats;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/libraries/info/query.rs` around lines 65 - 109, The code duplicates the library.recalculate_statistics().await call in both the if is_stale and else branches; refactor by keeping the distinct tracing::info (for stale) and tracing::debug (for non-stale) messages inside the if/else but move the library.recalculate_statistics().await call (with its warn-on-error handling) once after the if/else so it always runs and cached_stats is returned; ensure you reference the same cached_stats return value and preserve the error logging (the warn() block) around library.recalculate_statistics().
66-72: Nit: multi-sentence rationale comment.This block is 6 sentences. Per guidelines, inline comments should be kept to one sentence when possible. The NAS/closure-table rationale is valuable, but can be tightened.
As per coding guidelines: "Inline comments should explain WHY decisions were made, not WHAT the code does, and should be kept to one sentence when possible."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/ops/libraries/info/query.rs` around lines 66 - 72, Tighten the multi-sentence rationale comment above the calculate_file_statistics call to a single concise sentence that explains the why: e.g., note that we return zeros and spawn a background task to avoid blocking the RPC/UI on large libraries (closure-table walks can take minutes) and mention that the background task emits a ResourceChanged event when finished; update the comment block that currently spans multiple sentences to this single-line rationale referencing calculate_file_statistics and ResourceChanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/src/ops/libraries/info/query.rs`:
- Around line 59-64: The current check around cached_stats (config.statistics)
using is_stale conflates a legitimately empty library with "never calculated"
because it only tests zero counts; change config.statistics to carry an explicit
sentinel (e.g. Option<Statistics> or add a last_calculated_at/timestamp) and
update the libraries.info logic that currently computes is_stale to instead
check that sentinel (e.g. statistics.is_none() or timestamp is missing/too old)
before spawning recalculate_statistics; update references to cached_stats,
is_stale, config.statistics and the recalculate_statistics call so only
genuinely uncomputed or expired stats trigger background recomputation.
---
Nitpick comments:
In `@core/src/ops/libraries/info/query.rs`:
- Line 11: The `use tracing;` import is redundant because macros are already
invoked via fully-qualified paths (e.g., tracing::info!); remove the bare `use
tracing;` line or replace it with a selective import like `use tracing::{debug,
info, warn};` and then call `info!`, `debug!`, `warn!` unqualified in functions
such as those that currently call `tracing::info!`, ensuring imports match the
macros actually used (update any macro calls if you switch to unqualified
names).
- Around line 65-109: The code duplicates the
library.recalculate_statistics().await call in both the if is_stale and else
branches; refactor by keeping the distinct tracing::info (for stale) and
tracing::debug (for non-stale) messages inside the if/else but move the
library.recalculate_statistics().await call (with its warn-on-error handling)
once after the if/else so it always runs and cached_stats is returned; ensure
you reference the same cached_stats return value and preserve the error logging
(the warn() block) around library.recalculate_statistics().
- Around line 66-72: Tighten the multi-sentence rationale comment above the
calculate_file_statistics call to a single concise sentence that explains the
why: e.g., note that we return zeros and spawn a background task to avoid
blocking the RPC/UI on large libraries (closure-table walks can take minutes)
and mention that the background task emits a ResourceChanged event when
finished; update the comment block that currently spans multiple sentences to
this single-line rationale referencing calculate_file_statistics and
ResourceChanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: afbf3ce1-24ed-4d49-bdec-1d8663e4433f
📒 Files selected for processing (1)
core/src/ops/libraries/info/query.rs
Core::new() registers default protocol handlers after starting networking, but swallows any failure (error is only logged). If the initial registration fails — e.g. on a host where start_networking hasn't fully set up the event loop command sender by the time register_default_protocol_handlers runs — the registry is left empty. A subsequent call to Core::init_networking() would see `services.networking().is_some()` and skip re-registration, permanently leaving protocols unregistered for the life of the process. sd-server calls init_networking() right after Core::new(), so it's the client most exposed to this. Symptom: pairing over the web UI returns "Pairing protocol not registered" while the same library works fine from Tauri and mobile. Fix: init_networking now queries the registry directly for the pairing handler and re-registers the default set if it's missing, independent of whether networking is already initialized. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/lib.rs`:
- Around line 523-544: Replace the hardcoded "pairing" literal used when probing
the registry with the canonical name provider to avoid duplication and drift:
call PairingProtocolHandler::protocol_name() (or a shared constant) wherever
"pairing" is used in this file (the probe using
networking_service.protocol_registry().read().await.get_handler("pairing").is_some()
and the other occurrence noted in the review) so the registry lookup and any
future checks use the same authoritative symbol; additionally consider making
register_handler idempotent or explicitly check each default handler in
register_default_protocols to tolerate partial registrations (see
register_default_protocols and register_handler for the recovery path).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| // Core::new() attempts to register default protocols after starting | ||
| // networking, but swallows errors (only logs them). That means | ||
| // `already_initialized` does NOT imply protocols are registered — | ||
| // e.g. if the event loop's command sender wasn't ready when Core::new() | ||
| // tried to build the pairing handler, registration silently failed. | ||
| // Check the registry directly and re-register if missing. | ||
| let pairing_registered = networking_service | ||
| .protocol_registry() | ||
| .read() | ||
| .await | ||
| .get_handler("pairing") | ||
| .is_some(); | ||
|
|
||
| if !already_initialized || !pairing_registered { | ||
| if already_initialized && !pairing_registered { | ||
| logger | ||
| .warn("Networking was initialized but protocol handlers are missing; re-registering") | ||
| .await; | ||
| } else { | ||
| logger.info("Registering protocol handlers...").await; | ||
| } | ||
| self.register_default_protocols(&networking_service).await?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Confirm register_default_protocol_handlers is still all-or-nothing
# (no fallible step between successive register_handler calls).
rg -nP -C2 '\bregister_handler\s*\(' core/src
# 2) Find all occurrences of the "pairing" string literal used as a protocol name
rg -nP '"pairing"' core/src
# 3) Locate PairingProtocolHandler::protocol_name to confirm the canonical name
ast-grep --pattern $'impl $_ for PairingProtocolHandler {
$$$
fn protocol_name($$$) $$$ {
$$$
}
$$$
}'Repository: spacedriveapp/spacedrive
Length of output: 44054
Replace hardcoded "pairing" strings with a shared constant or PairingProtocolHandler::protocol_name() for robustness.
The all-or-nothing invariant currently holds: if command_sender acquisition fails at line 654–657, no handlers register at all, so probing for "pairing" correctly indicates whether all defaults are missing. However, this coupling is implicit and fragile. A future change that inserts a fallible operation between successive register_handler calls (lines 750–754) would silently break the assumption and cause a partial-registration state, leaving the registry inconsistent.
Two improvements:
-
Replace the magic string
"pairing"at lines 533 and 782 withPairingProtocolHandler::protocol_name()(defined incore/src/service/network/protocol/pairing/mod.rs:2138) or a shared module constant. This keeps the protocol name in sync with its canonical definition and reduces duplication across the codebase. -
Optionally, make
register_handlerskip-if-already-registered semantics for this recovery path, or check each default handler name explicitly, to harden against future partial-registration scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/src/lib.rs` around lines 523 - 544, Replace the hardcoded "pairing"
literal used when probing the registry with the canonical name provider to avoid
duplication and drift: call PairingProtocolHandler::protocol_name() (or a shared
constant) wherever "pairing" is used in this file (the probe using
networking_service.protocol_registry().read().await.get_handler("pairing").is_some()
and the other occurrence noted in the review) so the registry lookup and any
future checks use the same authoritative symbol; additionally consider making
register_handler idempotent or explicitly check each default handler in
register_default_protocols to tolerate partial registrations (see
register_default_protocols and register_handler for the recovery path).
Iroh's endpoint.bind() fails wholesale if any configured discovery service fails to initialize. MdnsDiscovery requires binding UDP :5353, which on most Linux systems (including TrueNAS) is already owned by avahi-daemon. Result: endpoint creation errors out with "Service 'mdns' error", the event loop never starts, command_sender stays None, and protocol registration fails — so sd-server has no working networking at all. Make mDNS best-effort: on any error whose message mentions "mdns", retry endpoint creation with only pkarr + DNS discovery. Local-network auto-discovery is lost but remote pairing via node ID (which uses n0's DNS infrastructure, not mDNS) continues to work normally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The dual-path discovery in start_pairing_as_joiner_with_code used tokio::select! to race mDNS and relay. select! resolves on the first branch to complete — including errors — so a host that can't bind mDNS (e.g. a Linux box where avahi already owns UDP :5353) would fail pairing wholesale: mDNS discovery errors out in <1ms with "Failed to create mDNS discovery: Service 'mdns' error", that Err wins the race, and relay discovery gets cancelled before it can even begin. Switch to futures::select_ok so we only return the error if EVERY discovery path has failed. mDNS failing immediately now leaves relay running to completion, which is the common case for remote pairing into a NAS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve conflicts: - migration/mod.rs: keep redundancy + tags + sync cursor migrations in chronological order (m20260125 → m20260414 → m20260417). - ExplorerView.tsx: render unless mode is plain "browse" without a path, covering both filtered (redundancy) and tag/recents modes. - context.tsx: combine ExplorerMode/Action/dispatcher additions for both filtered and tag modes side by side. - useExplorerFiles.ts: support both filtered and tag query paths with priority filtered > tag > recents > search > virtual > directory. - types.ts: keep Redundancy summary types alongside upstream changes.
…app#3053) * feat: add redundancy awareness & cross-volume file comparison Surface content redundancy data so users can answer "if this drive dies, what do I lose?" — builds on existing content identity and volume systems. Backend: - New `redundancy.summary` library query with per-volume at-risk vs redundant byte/file counts and a library-wide replication score - Extend `SearchFilters` with `at_risk`, `on_volumes`, `not_on_volumes`, `min_volume_count`, `max_volume_count` filters - Add composite index migration on entries(content_id, volume_id) Frontend: - `/redundancy` dashboard with replication score, volume bars, at-risk callout - `/redundancy/at-risk` paginated file list sorted by size - `/redundancy/compare` two-volume comparison (unique/shared toggle) - Sidebar ShieldCheck button linking to redundancy view Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * redundancy UI improvements + ZFS volume detection fix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix ZFS pool capacity reporting and stats filtering - ZFS: override total_capacity for pool-root volumes using zfs list used+available. df under-reports pool-root Size because it only counts the root dataset's own used bytes plus avail — on a 60 TB raidz2 pool this shows as ~15 TB instead of ~62 TB. The pool root's own used property includes descendants, so used+available is the real usable capacity. - Library stats: drop volumes where is_user_visible=false AND re-apply should_hide_by_mount_path retroactively so stale DB rows (detected before the Linux visibility filters existed) don't inflate reported capacity. - Extract should_hide_by_mount_path into volume/utils as a shared helper used by both the list query and the stats calculation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * show capacity and visibility in sd volume list Makes it possible to verify library-level capacity aggregation from the CLI — previously the list only showed mount, fingerprint, and tracked/mounted state, which meant debugging the ZFS pool capacity issue required querying the library DB directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * document filesystem support matrix and detection New docs/core/filesystems.mdx covering per-filesystem capabilities (CoW, pool-awareness, visibility filtering, capacity correction), platform detection strategies, the FilesystemHandler trait, Linux/ macOS/ZFS visibility rules, the ZFS pool-root capacity problem and fix, copy strategy selection, and known limitations. Registered under File Management in both mint.json and docs.json. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * WIP: redundancy filter wiring across search, CLI, and UI - core/src/ops/search: wire redundancy filters (at_risk, on_volumes, not_on_volumes, min/max volume_count) through the search query; fix UUID-to-SQLite BLOB literal so volume UUID comparisons actually match (volumes.uuid is stored as a 16-byte BLOB, quoted-string comparison silently returned zero rows). - apps/cli: new redundancy subcommand + populate the new SearchFilters fields from search args. - packages/interface: redundancy at-risk and compare pages reworked to consume the new filter surface; explorer context/hook updates to support redundancy-scoped views. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * add web context menu renderer and UI polish - New WebContextMenuProvider + Radix DropdownMenu-based renderer anchored at cursor via a 1x1 virtual trigger. Handles separators, submenus, disabled, and the danger variant via text-status-error. - useContextMenu now routes web clicks through the provider instead of parking data in unused local state, and trims leading/trailing/adjacent separators so condition-filtered menus don't render orphaned lines. - Drop app-frame corner rounding on the web build. - Add shrink-0 to the sidebar space switcher so the scrollable sibling can't compress it vertically. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * sd-server dev workflow: auto web build, shutdown watchdog, stable data dir - build.rs runs `bun run build` in apps/web so `just dev-server` always embeds the latest UI. rerun-if-changed covers apps/web/src, packages/interface/src, and packages/ts-client/src so Rust-only edits skip the rebuild. Skips gracefully when bun isn't on PATH or SD_SKIP_WEB_BUILD is set; Dockerfile sets the latter since dist is pre-built and bun isn't in the Rust stage. - Graceful shutdown was hanging because the browser holds the /events SSE stream open forever and axum waits for all connections to drain. After the first signal, arm a background force-exit on second Ctrl+C or 5s timeout so the process can't stick. - Debug builds were starting from a fresh tempfile::tempdir() on every run (the TempDir handle dropped at end of the closure, deleting the dir we just took a path to). Default to ~/.spacedrive in debug so data persists and `just dev-server` shares a data dir with the Tauri app. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * add Sources space item alongside Redundancy in default library layout Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * add TrueNAS native build script Uses zig cc as C/C++ compiler on TrueNAS Scale where /usr is read-only and no system gcc exists. Dev tools live at /mnt/pool/dev-tools/ (zig, cmake, make, extracted deb headers). Builds sd-server + sd-cli in ~4 min on a 12-core NAS. AI feature disabled (whisper.cpp C11 atomics incompatible with zig clang-18). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * never block RPC on synchronous statistics calculation On first load (fresh library, all stats zero), libraries.info used to calculate statistics synchronously before responding. On large libraries during active indexing this hangs indefinitely — the closure-table walk in calculate_file_statistics loads every descendant ID into a Vec then issues a WHERE IN(...) with millions of entries, which SQLite can't finish while the indexer is writing. Now always return cached (possibly zero) stats and let the background recalculate_statistics task fill them in. The UI refreshes via the ResourceChanged event when the calculation completes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * self-heal protocol handler registration on re-init Core::new() registers default protocol handlers after starting networking, but swallows any failure (error is only logged). If the initial registration fails — e.g. on a host where start_networking hasn't fully set up the event loop command sender by the time register_default_protocol_handlers runs — the registry is left empty. A subsequent call to Core::init_networking() would see `services.networking().is_some()` and skip re-registration, permanently leaving protocols unregistered for the life of the process. sd-server calls init_networking() right after Core::new(), so it's the client most exposed to this. Symptom: pairing over the web UI returns "Pairing protocol not registered" while the same library works fine from Tauri and mobile. Fix: init_networking now queries the registry directly for the pairing handler and re-registers the default set if it's missing, independent of whether networking is already initialized. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fall back to pkarr+DNS discovery when mDNS port is unavailable Iroh's endpoint.bind() fails wholesale if any configured discovery service fails to initialize. MdnsDiscovery requires binding UDP :5353, which on most Linux systems (including TrueNAS) is already owned by avahi-daemon. Result: endpoint creation errors out with "Service 'mdns' error", the event loop never starts, command_sender stays None, and protocol registration fails — so sd-server has no working networking at all. Make mDNS best-effort: on any error whose message mentions "mdns", retry endpoint creation with only pkarr + DNS discovery. Local-network auto-discovery is lost but remote pairing via node ID (which uses n0's DNS infrastructure, not mDNS) continues to work normally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * succeed pairing if either mDNS or relay discovery wins The dual-path discovery in start_pairing_as_joiner_with_code used tokio::select! to race mDNS and relay. select! resolves on the first branch to complete — including errors — so a host that can't bind mDNS (e.g. a Linux box where avahi already owns UDP :5353) would fail pairing wholesale: mDNS discovery errors out in <1ms with "Failed to create mDNS discovery: Service 'mdns' error", that Err wins the race, and relay discovery gets cancelled before it can even begin. Switch to futures::select_ok so we only return the error if EVERY discovery path has failed. mDNS failing immediately now leaves relay running to completion, which is the common case for remote pairing into a NAS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
redundancy.summaryquery endpoint returning per-volume at-risk vs redundant bytes/files and a library-wide replication score (0–1)search.fileswith 5 new redundancy filters:at_risk,on_volumes,not_on_volumes,min_volume_count,max_volume_count/redundancydashboard,/redundancy/at-riskfile list, and/redundancy/comparetwo-volume comparison UIentries(content_id, volume_id)for query performanceTest plan
idx_entries_content_volumeexists viaPRAGMA index_list('entries')redundancy.summarywith empty library (all zeros), single volume (100% at-risk), two volumes with overlap (mixed scores)search.fileswithat_risk: true— verify only single-volume files returnedsearch.fileswithon_volumes: [A], not_on_volumes: [B]— verify correct diff/redundancy— dashboard loads with score, volume bars, at-risk callout🤖 Generated with Claude Code
Note
Backend implementation adds a new
redundancyquery module with summary statistics and extends search filters to surface redundancy awareness. A composite database index optimizes the GROUP BY patterns needed for redundancy calculations. Frontend routes for dashboard, at-risk files, and cross-volume comparison are included alongside UI components for displaying replication scores and volume comparisons. All changes integrate with existing content identity and volume tracking systems without architectural modifications.Written by Tembo for commit 1eea61b. This will update automatically on new commits.