feat(web): Add persistence to unified instance filter in sidebar

[drtaru]

Persist unified instance scope filter across navigation

Saves the selected unified instance scope to localStorage (qui-unified-instance-filter) so it is restored when navigating back to the unified view after switching elsewhere in the sidebar.

Clicking "All active" clears the saved filter and resets to all instances.

Sidebar badge: Changed from two separate badges [3 active] [2/3] to a single combined [2/3 active] badge. The badge remains visible even when a single instance is selected, so the filter state is always visible without needing to be on the unified view.

Input needed:

It might make sense to remove the word "active" to prevent confusion, however the individual instance statuses are displayed just below with a connectivity dot so that may not matter.
Happy to include that modification in this PR if it makes sense.

Summary by CodeRabbit

• New Features

  • Unified instance selections are now persisted in browser storage and automatically restored across Header, Sidebar, Mobile navigation and Torrents views.

• Bug Fixes / UX

  • UI elements (checkboxes, counts, links, dropdowns) consistently reflect the persisted scope with sensible fallbacks; applying a scope saves it for future sessions and no longer encodes scope into the URL.

• Chores

  • Removed legacy URL search param and updated related search payload shapes to rely on the persisted filter.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Replaces route-encoded unified instance scope with a persisted localStorage-backed hook. Adds usePersistedUnifiedInstanceFilter, integrates it into Header, Sidebar, MobileFooterNav, Torrents and routes, removes URL instanceIds handling and the exported resolveUnifiedInstanceIds/search param constant, and updates UI and navigation to use the persisted normalized IDs.

Changes

Cohort / File(s)	Summary
Persistence Hook web/src/hooks/usePersistedUnifiedInstanceFilter.ts	New hook using useSyncExternalStore + localStorage key "qui-unified-instance-filter", parse/encode helpers, change-event dispatch, safe read/write and saveFilter callback.
Layout: Header web/src/components/layout/Header.tsx	Wires in persisted filter, computes effective unified IDs from persisted state (fallback to activeInstanceIds), persists on apply, removes route-encoded param usage and encoding/decoding logic, updates deps.
Layout: Mobile Footer Nav web/src/components/layout/MobileFooterNav.tsx	Reads persisted filter for effective IDs, saves normalized IDs on apply, stops encoding unified IDs into route search, updates checkbox/label UI logic.
Layout: Sidebar web/src/components/layout/Sidebar.tsx	Uses persisted filter (fallback to active IDs) for displayed unified IDs, saves on apply, removes URL-encoded unified-id logic and some inline badge rendering, updates dropdown checkbox states and active-count text.
Pages: Torrents web/src/pages/Torrents.tsx	Removes instanceIds from TorrentsProps search/onSearchChange, consumes persisted unified filter for unified scope, adjusts deep-link fetch to use activeInstanceIds when cross-instance, simplifies some title-speed logic.
Routes: Instances pages web/src/routes/_authenticated/instances.$instanceId.tsx, web/src/routes/_authenticated/instances.index.tsx	Removed instanceIds from search schemas and handler types; no other control-flow changes.
Lib: instances web/src/lib/instances.ts	Removed exported UNIFIED_INSTANCE_IDS_SEARCH_PARAM constant and resolveUnifiedInstanceIds function; retained parse/normalize/encode helpers.

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant UI as Header/Sidebar/MobileFooterNav
    participant Hook as usePersistedUnifiedInstanceFilter
    participant Storage as localStorage
    Note over UI,Hook: User selects or applies a unified scope
    User->>UI: select scope / apply
    UI->>Hook: saveFilter(normalizedIds)
    Hook->>Storage: write key "qui-unified-instance-filter"
    Hook->>Hook: dispatch "qui-unified-instance-filter-changed"
    Hook->>UI: useSyncExternalStore notifies subscribers (snapshot)
    UI->>UI: recompute displayedUnifiedInstanceIds and update UI

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(torrents): add unified cross-instance torrent table #1481 — Reworked unified-instance handling and route-encoded unified IDs that this change replaces.
• feat(details): torrent details panel improvements #884 — Modifies web/src/pages/Torrents.tsx search/props and navigation handling; overlaps with removal of instanceIds.
• fix(torrents): tighten filter sidebar hover feedback #518 — Changes to TorrentsProps and search-related surface; touches the same exported component signature.

Suggested labels

enhancement, web, area/frontend, mobile

Poem

🐰 I hopped through keys and stored each chosen id,
In burrows of JSON where tiny bytes hid.
I tuck your scope safe, then give it back at dawn,
A nibble, a hop — your filters carry on. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: adding persistence to the unified instance filter. This matches the primary objective and the core modifications across Header, MobileFooterNav, Sidebar, and the new usePersistedUnifiedInstanceFilter hook.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 152-156: Introduce a displayedUnifiedInstanceIds derived value
that returns the persistedUnifiedFilter-derived normalized IDs
(persistedNormalizedIds) when no URL scope exists, otherwise returns
effectiveUnifiedInstanceIds, and replace uses of effectiveUnifiedInstanceIds in
checkbox checked state and in toggleUnifiedScopeInstance baseline logic so the
scope menu and badge share the same source of truth; update Sidebar.tsx (and
mirror the same change in Header.tsx and MobileFooterNav.tsx) so the badge
counting, the checkbox state, and the toggleUnifiedScopeInstance function all
use displayedUnifiedInstanceIds instead of effectiveUnifiedInstanceIds.

In `@web/src/hooks/usePersistedUnifiedInstanceFilter.ts`:
- Around line 15-36: The hook usePersistedUnifiedInstanceFilter currently uses
per-component useState (persistedIds) so updates in one component don't
propagate to others; replace the local state with a shared subscription using
useSyncExternalStore: implement a getSnapshot that reads STORAGE_KEY and returns
parseUnifiedInstanceIds(localStorage.getItem(STORAGE_KEY)) (or []), implement a
subscribe function that listens to the window "storage" event and a custom
same-tab event (e.g. "unified-instance-filter-changed") and returns an
unsubscribe, then change saveFilter to write encodeUnifiedInstanceIds to
localStorage (or removeItem), and dispatch the same-tab custom event so
subscribers (via useSyncExternalStore) see immediate updates; keep
parseUnifiedInstanceIds, encodeUnifiedInstanceIds, STORAGE_KEY,
usePersistedUnifiedInstanceFilter, and saveFilter names to locate the code.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f0946ade-73f1-42b2-a5a1-71a94200a40e

📥 Commits

Reviewing files that changed from the base of the PR and between 8713667 and 041e7c1.

📒 Files selected for processing (4)

• web/src/components/layout/Header.tsx
• web/src/components/layout/MobileFooterNav.tsx
• web/src/components/layout/Sidebar.tsx
• web/src/hooks/usePersistedUnifiedInstanceFilter.ts

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

web/src/components/layout/Header.tsx (1)

111-120: Consider extracting shared persistence logic into the hook.

All three layout components (Header, Sidebar, MobileFooterNav) duplicate the same derived state pattern:

const persistedNormalizedIds = useMemo(
  () => normalizeUnifiedInstanceIds(persistedUnifiedFilter, activeInstanceIds),
  [persistedUnifiedFilter, activeInstanceIds]
)
const displayedUnifiedInstanceIds = hasCustomUnifiedScope
  ? effectiveUnifiedInstanceIds
  : persistedNormalizedIds.length > 0
    ? persistedNormalizedIds
    : effectiveUnifiedInstanceIds

This could be encapsulated in the hook by passing activeInstanceIds and the URL-derived scope, returning both persistedIds and displayedIds. However, this is a nice-to-have for future cleanup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/Header.tsx` around lines 111 - 120, The layout
components (Header, Sidebar, MobileFooterNav) duplicate logic that computes
persistedNormalizedIds and displayedUnifiedInstanceIds; move this derived-state
into the usePersistedUnifiedInstanceFilter hook by accepting activeInstanceIds
and the URL-derived scope (hasCustomUnifiedScope/effectiveUnifiedInstanceIds)
and returning both persistedIds (result of normalizeUnifiedInstanceIds) and
displayedIds (the conditional choosing between persistedIds and
effectiveUnifiedInstanceIds). Update callers to replace local
persistedNormalizedIds and displayedUnifiedInstanceIds with the hook's returned
values to eliminate duplication and centralize normalization logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/hooks/usePersistedUnifiedInstanceFilter.ts`:
- Around line 43-55: The event dispatch is outside the try/catch so CHANGE_EVENT
is fired even when localStorage write/remove fails; move
window.dispatchEvent(new Event(CHANGE_EVENT)) inside the try block in the
saveFilter callback (the function named saveFilter in
usePersistedUnifiedInstanceFilter.ts) so the change event is only emitted after
successful encodeUnifiedInstanceIds and localStorage.setItem/removeItem, and
keep the catch to log the error without dispatching the event.

---

Nitpick comments:
In `@web/src/components/layout/Header.tsx`:
- Around line 111-120: The layout components (Header, Sidebar, MobileFooterNav)
duplicate logic that computes persistedNormalizedIds and
displayedUnifiedInstanceIds; move this derived-state into the
usePersistedUnifiedInstanceFilter hook by accepting activeInstanceIds and the
URL-derived scope (hasCustomUnifiedScope/effectiveUnifiedInstanceIds) and
returning both persistedIds (result of normalizeUnifiedInstanceIds) and
displayedIds (the conditional choosing between persistedIds and
effectiveUnifiedInstanceIds). Update callers to replace local
persistedNormalizedIds and displayedUnifiedInstanceIds with the hook's returned
values to eliminate duplication and centralize normalization logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 14b47286-896d-4f23-9a6a-e4de865f0a73

📥 Commits

Reviewing files that changed from the base of the PR and between 041e7c1 and 044e612.

📒 Files selected for processing (4)

• web/src/components/layout/Header.tsx
• web/src/components/layout/MobileFooterNav.tsx
• web/src/components/layout/Sidebar.tsx
• web/src/hooks/usePersistedUnifiedInstanceFilter.ts

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

web/src/components/layout/Sidebar.tsx (1)

135-165: Consider extracting the unified-scope state into a shared hook.

This normalize/fallback/apply/toggle flow now lives here, in web/src/components/layout/MobileFooterNav.tsx, and in Header.tsx. Pulling it into one hook/helper would make future scope/badge changes land once and reduce drift between the three menus.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/Sidebar.tsx` around lines 135 - 165, Extract the
unified-scope logic duplicated across Sidebar.tsx, MobileFooterNav.tsx, and
Header.tsx into a shared hook (e.g., useUnifiedScope) that encapsulates state
and helpers: usePersistedUnifiedInstanceFilter usage,
normalizeUnifiedInstanceIds fallback logic, effectiveUnifiedInstanceIds
calculation, hasCustomUnifiedScope, unifiedScopeSummary, applyUnifiedScope
(navigate+saveUnifiedFilter behavior) and toggleUnifiedScopeInstance; replace
the in-component implementations in Sidebar (functions/variables:
usePersistedUnifiedInstanceFilter, normalizeUnifiedInstanceIds,
effectiveUnifiedInstanceIds, applyUnifiedScope, toggleUnifiedScopeInstance) with
calls to the new hook so all three components consume the same centralized logic
and reduce duplication.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/pages/Torrents.tsx`:
- Around line 125-152: The deep-link lookup in fetchTorrentByHash is incorrectly
limited to unifiedScopeInstanceIds, causing /instances?torrent=... to fail when
the owning instance isn't in saved scope; change the logic so that when
resolving a hash (in fetchTorrentByHash / isAllInstances branch using
api.getCrossInstanceTorrents) you query across all active instances (e.g., use
activeInstanceIds or omit instanceIds to search globally) or perform a fallback
unscoped cross-instance lookup if the first scoped call returns null, and only
then clear search params; ensure to reference api.getCrossInstanceTorrents,
unifiedScopeInstanceIds, isAllInstances and the fallback to
api.getTorrents(instanceId, ...) so the code will resolve deep links regardless
of the saved unified scope.

---

Nitpick comments:
In `@web/src/components/layout/Sidebar.tsx`:
- Around line 135-165: Extract the unified-scope logic duplicated across
Sidebar.tsx, MobileFooterNav.tsx, and Header.tsx into a shared hook (e.g.,
useUnifiedScope) that encapsulates state and helpers:
usePersistedUnifiedInstanceFilter usage, normalizeUnifiedInstanceIds fallback
logic, effectiveUnifiedInstanceIds calculation, hasCustomUnifiedScope,
unifiedScopeSummary, applyUnifiedScope (navigate+saveUnifiedFilter behavior) and
toggleUnifiedScopeInstance; replace the in-component implementations in Sidebar
(functions/variables: usePersistedUnifiedInstanceFilter,
normalizeUnifiedInstanceIds, effectiveUnifiedInstanceIds, applyUnifiedScope,
toggleUnifiedScopeInstance) with calls to the new hook so all three components
consume the same centralized logic and reduce duplication.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 09da2d23-6369-4c09-91da-48138c871ff7

📥 Commits

Reviewing files that changed from the base of the PR and between 3f1ff03 and 85c2d28.

📒 Files selected for processing (7)

• web/src/components/layout/Header.tsx
• web/src/components/layout/MobileFooterNav.tsx
• web/src/components/layout/Sidebar.tsx
• web/src/lib/instances.ts
• web/src/pages/Torrents.tsx
• web/src/routes/_authenticated/instances.$instanceId.tsx
• web/src/routes/_authenticated/instances.index.tsx

💤 Files with no reviewable changes (3)

• web/src/routes/_authenticated/instances.$instanceId.tsx
• web/src/lib/instances.ts
• web/src/routes/_authenticated/instances.index.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• web/src/components/layout/Header.tsx

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

web/src/pages/Torrents.tsx (1)

53-60: ⚠️ Potential issue | 🟠 Major

Reset unified-view state when the persisted scope changes.

unifiedScopeInstanceIds can now change without a route change, but this component still treats instanceId as the only page identity. In /instances, switching the saved scope can keep the details panel open for a torrent whose instance is no longer in scope, and the sidebar counts stay marked as fresh until new data lands. Please treat the normalized unified scope as part of this page's identity as well, preferably with a key or render-time exclusion instead of an Effect.

As per coding guidelines, "To reset state in React, prefer a key or render-time adjustments instead of Effects".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web/src/pages/Torrents.tsx` around lines 53 - 60, The page must treat the
normalized unified scope as part of its identity so state resets when
persistedUnifiedFilter changes: compute unifiedScopeInstanceIds (using
normalizeUnifiedInstanceIds with persistedUnifiedFilter and activeInstanceIds)
as you already do, then include that value in the component tree key (e.g., the
root/wrapper component rendered by Torrents.tsx) so React remounts when
unifiedScopeInstanceIds changes; use a stable serialization (JSON.stringify or
join) of unifiedScopeInstanceIds combined with instanceId/isAllInstances to form
the key and remove any Effect-based state reset logic that attempted to watch
persistedUnifiedFilter.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/pages/Torrents.tsx`:
- Around line 125-140: The deep-link fetch (fetchTorrentByHash / the
cross-instance branch in the useEffect that reads search.torrent) must be gated
until useInstances has finished loading: check the isLoading flag (from
useInstances) and do not execute the cross-instance API call when isLoading is
true (or when activeInstanceIds is empty while loading); this prevents sending
instanceIds: undefined and consuming the deep link prematurely—either skip the
fetch while isLoading and let the effect re-run when instances resolve, or
retain search.torrent and explicitly retry when activeInstanceIds changes after
isLoading becomes false.

---

Outside diff comments:
In `@web/src/pages/Torrents.tsx`:
- Around line 53-60: The page must treat the normalized unified scope as part of
its identity so state resets when persistedUnifiedFilter changes: compute
unifiedScopeInstanceIds (using normalizeUnifiedInstanceIds with
persistedUnifiedFilter and activeInstanceIds) as you already do, then include
that value in the component tree key (e.g., the root/wrapper component rendered
by Torrents.tsx) so React remounts when unifiedScopeInstanceIds changes; use a
stable serialization (JSON.stringify or join) of unifiedScopeInstanceIds
combined with instanceId/isAllInstances to form the key and remove any
Effect-based state reset logic that attempted to watch persistedUnifiedFilter.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7a762306-140c-4f19-a61a-6c5c46609973

📥 Commits

Reviewing files that changed from the base of the PR and between 85c2d28 and 4d4dbfe.

📒 Files selected for processing (1)

• web/src/pages/Torrents.tsx