From 7f531e18c83a3d25534998eadb9114fc68d1e08a Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:00:20 +0530 Subject: [PATCH 1/9] feat(settings): add a Memories group to user preferences Collapsible group beside Video Tagging, holding the generate and desktop notification toggles plus dropdowns for seconds per photo and the photo count bounds. The min and max pair still cross-clamps, since a value saved by the old sliders can fall outside these options. --- .../components/UserPreferencesCard.tsx | 246 ++++++++++++++++++ 1 file changed, 246 insertions(+) diff --git a/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx b/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx index 2822163f9..7c1dcf7d9 100644 --- a/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx +++ b/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx @@ -6,6 +6,8 @@ import { Trash2, Clapperboard, HardDrive, + Sparkles, + Bell, } from 'lucide-react'; import { Label } from '@/components/ui/label'; @@ -24,6 +26,7 @@ import { listen } from '@tauri-apps/api/event'; import { getCurrentWindow } from '@tauri-apps/api/window'; import { useUserPreferences } from '@/hooks/useUserPreferences'; +import type { UpdateUserPreferencesRequest } from '@/api/api-functions/user_preferences'; import { purgeVideoFrameCache } from '@/api/api-functions'; import SettingsCard from './SettingsCard'; import { cn, formatTierLabel } from '@/lib/utils'; @@ -40,13 +43,20 @@ import { */ // Coarse enough to be a meaningful cost tradeoff, fine enough to matter. const FRAME_INTERVAL_OPTIONS = [2, 5, 10, 30]; +// Story pacing: below 3s a photo barely registers, above 10s it drags. +const SLIDE_DURATION_OPTIONS = [3, 5, 7, 10]; +const MIN_IMAGE_OPTIONS = [3, 5, 8, 10]; +const MAX_IMAGE_OPTIONS = [20, 30, 50, 100]; const UserPreferencesCard: React.FC = () => { const { preferences, + memoriesPreferences, updateYoloModelSize, toggleGpuAcceleration, updateVideoFrameInterval, + updateMemoriesPreferences, + isUpdating, refetch, } = useUserPreferences(); const [installedTiers, setInstalledTiers] = useState([]); @@ -58,6 +68,14 @@ const UserPreferencesCard: React.FC = () => { // Collapsed by default: video tagging is a niche setting, so it stays out // of the way until a user with videos goes looking for it. const [videoSettingsOpen, setVideoSettingsOpen] = useState(false); + const [memorySettingsOpen, setMemorySettingsOpen] = useState(false); + + const patchMemories = useCallback( + (update: UpdateUserPreferencesRequest['memories']) => { + void updateMemoriesPreferences(update).catch(console.warn); + }, + [updateMemoriesPreferences], + ); const handlePurgeFrameCache = useCallback(async () => { setPurgeState('purging'); @@ -360,6 +378,234 @@ const UserPreferencesCard: React.FC = () => { )} + + {/* Memories: same collapsible treatment as video tagging, so the + two niche groups read as siblings. */} +
+ + + {memorySettingsOpen && ( +
+ {/* Generate Memories Setting */} +
+
+ +

+ Curate highlights from your library automatically. +

+
+
+ + + patchMemories({ enabled: checked }) + } + /> +
+
+ + {/* Desktop Notifications Setting */} +
+
+ +

+ Get an alert when a new memory is ready. +

+
+
+ + + patchMemories({ notifications_enabled: checked }) + } + /> +
+
+ + {/* Seconds Per Photo Setting */} +
+
+ +

+ How long each photo is held before the story advances. A + video clip runs for its own length instead. +

+
+ + + + + + {SLIDE_DURATION_OPTIONS.map((seconds) => ( + + patchMemories({ slide_duration_seconds: seconds }) + } + > + {seconds} seconds + + ))} + + +
+ + {/* Minimum Photos Setting */} +
+
+ +

+ Groups with fewer photos than this are skipped rather than + turned into a memory. +

+
+ + + + + + {MIN_IMAGE_OPTIONS.map((count) => ( + + patchMemories({ + min_images: count, + // Keep the pair valid: the backend rejects a + // minimum above the maximum, failing the save. + max_images: Math.max( + count, + memoriesPreferences.max_images, + ), + }) + } + > + {count} photos + + ))} + + +
+ + {/* Maximum Photos Setting */} +
+
+ +

+ No memory shows more photos than this, however many the + occasion produced. +

+
+ + + + + + {MAX_IMAGE_OPTIONS.map((count) => ( + + patchMemories({ + max_images: count, + min_images: Math.min( + count, + memoriesPreferences.min_images, + ), + }) + } + > + {count} photos + + ))} + + +
+
+ )} +
); From 46920bc29cb452554a7f371558e1f7573ba59362 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:01:00 +0530 Subject: [PATCH 2/9] refactor(memories): remove the standalone memory settings page Its controls now live in the settings page, so the page, the memories/settings route and ROUTES.MEMORIES_SETTINGS all go. The gear buttons on the grid and in the story viewer point at /settings instead. --- .../components/Memories/MemoryStoryViewer.tsx | 2 +- frontend/src/constants/routes.ts | 1 - frontend/src/pages/Memories/Memories.tsx | 2 +- .../src/pages/Memories/MemorySettings.tsx | 197 ------------------ frontend/src/routes/AppRoutes.tsx | 4 - 5 files changed, 2 insertions(+), 204 deletions(-) delete mode 100644 frontend/src/pages/Memories/MemorySettings.tsx diff --git a/frontend/src/components/Memories/MemoryStoryViewer.tsx b/frontend/src/components/Memories/MemoryStoryViewer.tsx index a608c45f3..b8404ada1 100644 --- a/frontend/src/components/Memories/MemoryStoryViewer.tsx +++ b/frontend/src/components/Memories/MemoryStoryViewer.tsx @@ -291,7 +291,7 @@ export const MemoryStoryViewer: React.FC = ({ )} -

Memory settings

- - - - patch({ enabled: checked })} - /> -
- patch({ notifications_enabled: checked })} - /> - - - - patch({ story_music_enabled: checked })} - /> -
-
-
- - - {memoriesPreferences.slide_duration_seconds}s - -
-

- How long each photo is held before the story advances. -

- - patch({ slide_duration_seconds: value }) - } - /> -
- - - -
- -

- Memories with fewer than {memoriesPreferences.min_images} photos - are skipped, and no memory shows more than{' '} - {memoriesPreferences.max_images}. -

-
-
-
-
- - - {memoriesPreferences.min_images} - -
- - patch({ - min_images: value, - // Keep the pair valid: the backend rejects a minimum above - // the maximum, which would fail the whole save. - max_images: Math.max(value, memoriesPreferences.max_images), - }) - } - /> -
-
-
- - - {memoriesPreferences.max_images} - -
- - patch({ - max_images: value, - min_images: Math.min(value, memoriesPreferences.min_images), - }) - } - /> -
- -
-
- ); -}; - -export default MemorySettings; diff --git a/frontend/src/routes/AppRoutes.tsx b/frontend/src/routes/AppRoutes.tsx index cfca00e6a..267452dd5 100644 --- a/frontend/src/routes/AppRoutes.tsx +++ b/frontend/src/routes/AppRoutes.tsx @@ -11,7 +11,6 @@ import { AITagging } from '@/pages/AITagging/AITagging'; import { PersonImages } from '@/pages/PersonImages/PersonImages'; import { ComingSoon } from '@/pages/ComingSoon/ComingSoon'; import { Memories } from '@/pages/Memories/Memories'; -import { MemorySettings } from '@/pages/Memories/MemorySettings'; import { ModelManager } from '@/pages/ModelManager/ModelManager'; import { SearchResults } from '@/pages/SearchResults/SearchResults'; @@ -28,9 +27,6 @@ export const AppRoutes: React.FC = () => { } /> } /> } /> - {/* Declared before the index route so "memories/settings" is not - swallowed as a memory id. */} - } /> } /> } /> } /> From b7d0b7e6bbf58b733da7bfbaa4d88df680ede2dd Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:01:17 +0530 Subject: [PATCH 3/9] fix(memories): default desktop notifications to off Alerts are opt-in now; the memory is waiting on the page either way. Flipped in MemoriesPreferences, the MemoryStatusData echo and the frontend defaults, with openapi.json regenerated to match. --- backend/app/schemas/memories.py | 2 +- backend/app/schemas/user_preferences.py | 3 ++- backend/tests/test_user_preferences.py | 3 ++- docs/backend/backend_python/openapi.json | 4 ++-- frontend/src/hooks/useUserPreferences.tsx | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/backend/app/schemas/memories.py b/backend/app/schemas/memories.py index 2630347ae..2081c6ada 100644 --- a/backend/app/schemas/memories.py +++ b/backend/app/schemas/memories.py @@ -108,7 +108,7 @@ class MemoryStatusData(BaseModel): unviewed_count: int = 0 latest_memory_id: Optional[str] = None memories_enabled: bool = True - notifications_enabled: bool = True + notifications_enabled: bool = False class GetMemoriesData(BaseModel): diff --git a/backend/app/schemas/user_preferences.py b/backend/app/schemas/user_preferences.py index aaa8e2d23..9f00c94ac 100644 --- a/backend/app/schemas/user_preferences.py +++ b/backend/app/schemas/user_preferences.py @@ -45,7 +45,8 @@ class MemoriesPreferences(BaseModel): """Memory generation and delivery preferences.""" enabled: bool = True - notifications_enabled: bool = True + # Desktop alerts are opt-in; the memory is waiting on the page regardless. + notifications_enabled: bool = False # The story viewer ships muted; the user opts into background audio. story_music_enabled: bool = False # Seconds each photo is held before the story advances. A video slide diff --git a/backend/tests/test_user_preferences.py b/backend/tests/test_user_preferences.py index 4fbecb986..6ff03cb54 100644 --- a/backend/tests/test_user_preferences.py +++ b/backend/tests/test_user_preferences.py @@ -582,7 +582,8 @@ def test_defaults_appear_when_the_key_is_absent(self): memories = response.json()["user_preferences"]["memories"] assert memories["enabled"] is True - assert memories["notifications_enabled"] is True + # Desktop alerts are opt-in. + assert memories["notifications_enabled"] is False # The story viewer ships muted; audio is opt-in. assert memories["story_music_enabled"] is False assert memories["min_images"] == 5 diff --git a/docs/backend/backend_python/openapi.json b/docs/backend/backend_python/openapi.json index 0fdb46fe4..8690c089f 100644 --- a/docs/backend/backend_python/openapi.json +++ b/docs/backend/backend_python/openapi.json @@ -3721,7 +3721,7 @@ "notifications_enabled": { "type": "boolean", "title": "Notifications Enabled", - "default": true + "default": false }, "story_music_enabled": { "type": "boolean", @@ -4353,7 +4353,7 @@ "notifications_enabled": { "type": "boolean", "title": "Notifications Enabled", - "default": true + "default": false } }, "type": "object", diff --git a/frontend/src/hooks/useUserPreferences.tsx b/frontend/src/hooks/useUserPreferences.tsx index 2fd22aa64..c6cf37e35 100644 --- a/frontend/src/hooks/useUserPreferences.tsx +++ b/frontend/src/hooks/useUserPreferences.tsx @@ -11,7 +11,7 @@ import { useMutationFeedback } from './useMutationFeedback'; export const DEFAULT_MEMORIES_PREFERENCES: MemoriesPreferences = { enabled: true, - notifications_enabled: true, + notifications_enabled: false, story_music_enabled: false, slide_duration_seconds: 5, min_images: 5, From 02dc31c0c92e897b2cc4787440b915078e4762a2 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:15:20 +0530 Subject: [PATCH 4/9] fix(settings): disable memory dropdowns while a save is in flight The switches already did this, the three dropdown triggers did not. Rapid selections could overlap, and each mutation rolls back to its own snapshot, so an out-of-order failure could leave the min/max pair inconsistent. --- .../src/pages/SettingsPage/components/UserPreferencesCard.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx b/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx index 7c1dcf7d9..d7f3a7dd5 100644 --- a/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx +++ b/frontend/src/pages/SettingsPage/components/UserPreferencesCard.tsx @@ -487,6 +487,7 @@ const UserPreferencesCard: React.FC = () => { id="memories-duration" variant="outline" className="w-32 cursor-pointer justify-between" + disabled={isUpdating} > {memoriesPreferences.slide_duration_seconds}s @@ -528,6 +529,7 @@ const UserPreferencesCard: React.FC = () => { id="memories-min" variant="outline" className="w-32 cursor-pointer justify-between" + disabled={isUpdating} > {memoriesPreferences.min_images} photos @@ -577,6 +579,7 @@ const UserPreferencesCard: React.FC = () => { id="memories-max" variant="outline" className="w-32 cursor-pointer justify-between" + disabled={isUpdating} > {memoriesPreferences.max_images} photos From 4a5babe07072146f47f75daa91b4b3396d55d957 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:35:20 +0530 Subject: [PATCH 5/9] fix(settings): serialize preference writes and roll back per key Every write snapshotted and restored the whole preferences object, so two overlapping saves would have the later one roll back over the earlier one. Writes now queue, each builds from the state that actually landed before it, and each sends only the keys it changed so the server merge cannot clobber a concurrent edit either. Drops the unused updatePreference, which was the whole-object path. --- .../__tests__/useUserPreferences.test.tsx | 190 ++++++++++++++++++ frontend/src/hooks/useUserPreferences.tsx | 128 +++++++----- 2 files changed, 265 insertions(+), 53 deletions(-) create mode 100644 frontend/src/hooks/__tests__/useUserPreferences.test.tsx diff --git a/frontend/src/hooks/__tests__/useUserPreferences.test.tsx b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx new file mode 100644 index 000000000..f9a167742 --- /dev/null +++ b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx @@ -0,0 +1,190 @@ +import React from 'react'; +import { renderHook, act, waitFor } from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { configureStore } from '@reduxjs/toolkit'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; + +import { rootReducer } from '@/app/store'; +import { useUserPreferences } from '@/hooks/useUserPreferences'; +import type { + UpdateUserPreferencesRequest, + UserPreferencesData, +} from '@/api/api-functions/user_preferences'; + +const mockGetUserPreferences = jest.fn(); +const mockUpdateUserPreferences = jest.fn(); + +jest.mock('@/api/api-functions/user_preferences', () => ({ + getUserPreferences: () => mockGetUserPreferences(), + updateUserPreferences: (request: UpdateUserPreferencesRequest) => + mockUpdateUserPreferences(request), +})); + +const baseline: UserPreferencesData = { + // Differs from the hook's initial state, so mountLoaded can tell the two + // apart and does not race the load. + YOLO_model_size: 'medium', + GPU_Acceleration: false, + Video_Frame_Interval: 5, + memories: { + enabled: true, + notifications_enabled: false, + story_music_enabled: false, + slide_duration_seconds: 5, + min_images: 5, + max_images: 30, + weights: { + favourite: 0.22, + known_people: 0.2, + event_strength: 0.18, + face_presence: 0.12, + semantic_confidence: 0.1, + gps_novelty: 0.1, + in_album: 0.08, + }, + }, +}; + +/** Built once per test: a client rebuilt on every render remounts the query. */ +const makeWrapper = () => { + const store = configureStore({ reducer: rootReducer }); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); +}; + +/** Mount and wait for the initial load to land in local state. */ +const mountLoaded = async () => { + const { result } = renderHook(() => useUserPreferences(), { + wrapper: makeWrapper(), + }); + await waitFor(() => + expect(result.current.preferences.YOLO_model_size).toBe('medium'), + ); + return result; +}; + +const sentBodies = () => + mockUpdateUserPreferences.mock.calls.map(([body]) => body); + +beforeEach(() => { + mockGetUserPreferences + .mockReset() + // The refetch after a successful write would otherwise overwrite local + // state from the server and mask what optimistic apply and rollback did. + .mockResolvedValueOnce({ + success: true, + message: 'ok', + user_preferences: baseline, + }) + .mockImplementation(() => new Promise(() => {})); + + mockUpdateUserPreferences.mockReset().mockResolvedValue({ + success: true, + message: 'ok', + user_preferences: baseline, + }); +}); + +describe('useUserPreferences', () => { + it('sends only the keys it changed', async () => { + const result = await mountLoaded(); + + await act(async () => { + await result.current.updateVideoFrameInterval(10); + }); + + // A whole-object PUT would carry a concurrent edit's stale value along + // with it, so each write names only its own key. + expect(sentBodies()).toEqual([{ Video_Frame_Interval: 10 }]); + }); + + it('builds a queued write from what landed before it', async () => { + const result = await mountLoaded(); + + // Two toggles from the same render. Reading the render's snapshot rather + // than the applied value makes both compute the same "next" state. + await act(async () => { + const first = result.current.toggleGpuAcceleration(); + const second = result.current.toggleGpuAcceleration(); + await Promise.all([first, second]); + }); + + expect(sentBodies()).toEqual([ + { GPU_Acceleration: true }, + { GPU_Acceleration: false }, + ]); + expect(result.current.preferences.GPU_Acceleration).toBe(false); + }); + + it('rolls back only the write that failed', async () => { + mockUpdateUserPreferences.mockImplementation( + async (request: UpdateUserPreferencesRequest) => { + if (request.Video_Frame_Interval !== undefined) { + throw new Error('save failed'); + } + return { success: true, message: 'ok', user_preferences: baseline }; + }, + ); + + const result = await mountLoaded(); + + await act(async () => { + const memories = result.current.updateMemoriesPreferences({ + min_images: 8, + }); + const interval = result.current + .updateVideoFrameInterval(30) + .catch(() => undefined); + await Promise.all([memories, interval]); + }); + + // The failed write must restore its own key and leave the successful one + // alone. A snapshot captured before the memories write applied would + // revert min_images to 5 here. + await waitFor(() => + expect(result.current.preferences.Video_Frame_Interval).toBe(5), + ); + expect(result.current.memoriesPreferences.min_images).toBe(8); + }); + + it('keeps accepting writes after one fails', async () => { + mockUpdateUserPreferences.mockRejectedValueOnce(new Error('save failed')); + + const result = await mountLoaded(); + + await act(async () => { + await result.current.updateVideoFrameInterval(30).catch(() => undefined); + }); + + await act(async () => { + await result.current.updateVideoFrameInterval(10); + }); + + // A rejection left unhandled in the queue would stall every later write. + expect(result.current.preferences.Video_Frame_Interval).toBe(10); + }); + + it('merges a memories patch over the stored weights', async () => { + const result = await mountLoaded(); + + await act(async () => { + await result.current.updateMemoriesPreferences({ + weights: { favourite: 0.5 }, + }); + }); + + expect(result.current.memoriesPreferences.weights).toEqual({ + ...baseline.memories.weights, + favourite: 0.5, + }); + expect(sentBodies()).toEqual([ + { memories: { weights: { favourite: 0.5 } } }, + ]); + }); +}); diff --git a/frontend/src/hooks/useUserPreferences.tsx b/frontend/src/hooks/useUserPreferences.tsx index c6cf37e35..61563fa4c 100644 --- a/frontend/src/hooks/useUserPreferences.tsx +++ b/frontend/src/hooks/useUserPreferences.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useRef } from 'react'; import { usePictoMutation, usePictoQuery } from '@/hooks/useQueryExtension'; import { getUserPreferences, @@ -39,6 +39,15 @@ export const useUserPreferences = () => { memories: DEFAULT_MEMORIES_PREFERENCES, }); + // Writes read and roll back against this rather than `preferences`, which is + // a render-old snapshot for anything already in flight. + const preferencesRef = useRef(preferences); + + const applyPreferences = (next: UserPreferencesData) => { + preferencesRef.current = next; + setPreferences(next); + }; + // Query for user preferences const preferencesQuery = usePictoQuery({ queryKey: ['userPreferences'], @@ -51,8 +60,9 @@ export const useUserPreferences = () => { preferencesQuery.data?.success && preferencesQuery.data.user_preferences ) { - setPreferences(preferencesQuery.data.user_preferences); + applyPreferences(preferencesQuery.data.user_preferences); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [preferencesQuery.data]); // Mutation for updating user preferences @@ -75,78 +85,91 @@ export const useUserPreferences = () => { errorMessage: 'Failed to update preferences. Please try again.', }); + // One write at a time. Concurrent PUTs race on the server, and a rollback + // captured while another write is in flight restores that write's optimistic + // value rather than what is actually stored. + const writeQueue = useRef>(Promise.resolve()); + /** - * Update a specific preference + * Apply a change optimistically and send it, queued behind any write already + * running. + * + * `build` runs when the write reaches the front of the queue, not when it was + * requested, so a queued change is computed from what actually landed before + * it. It returns the full next state and the request body, which carries only + * the changed keys so a concurrent edit elsewhere in settings survives. */ - const updatePreference = async (updatedPreferences: UserPreferencesData) => { - const previousPreferences = preferences; - setPreferences(updatedPreferences); - try { - return await updatePreferencesMutation.mutateAsync(updatedPreferences); - } catch (err) { - setPreferences(previousPreferences); - throw err; - } + const writePreferences = ( + build: (current: UserPreferencesData) => { + next: UserPreferencesData; + request: UpdateUserPreferencesRequest; + }, + ) => { + const send = async () => { + const current = preferencesRef.current; + const { next, request } = build(current); + applyPreferences(next); + try { + return await updatePreferencesMutation.mutateAsync(request); + } catch (err) { + applyPreferences(current); + throw err; + } + }; + + const result = writeQueue.current.then(send, send); + // A rejected write must not stall every later one. + writeQueue.current = result.catch(() => undefined); + return result; }; /** * Update YOLO model size */ - const updateYoloModelSize = async (size: 'nano' | 'small' | 'medium') => { - const updatedPreferences = { - ...preferences, - YOLO_model_size: size, - }; - return updatePreference(updatedPreferences); - }; + const updateYoloModelSize = async (size: 'nano' | 'small' | 'medium') => + writePreferences((current) => ({ + next: { ...current, YOLO_model_size: size }, + request: { YOLO_model_size: size }, + })); /** * Toggle GPU acceleration */ - const toggleGpuAcceleration = async () => { - const updatedPreferences = { - ...preferences, - GPU_Acceleration: !preferences.GPU_Acceleration, - }; - return updatePreference(updatedPreferences); - }; + const toggleGpuAcceleration = async () => + writePreferences((current) => { + const GPU_Acceleration = !current.GPU_Acceleration; + return { + next: { ...current, GPU_Acceleration }, + request: { GPU_Acceleration }, + }; + }); /** * Update the video keyframe sampling interval (seconds) */ - const updateVideoFrameInterval = async (interval: number) => { - const updatedPreferences = { - ...preferences, - Video_Frame_Interval: interval, - }; - return updatePreference(updatedPreferences); - }; + const updateVideoFrameInterval = async (interval: number) => + writePreferences((current) => ({ + next: { ...current, Video_Frame_Interval: interval }, + request: { Video_Frame_Interval: interval }, + })); /** * Patch memories preferences. - * - * Sends only the changed keys rather than the whole preferences object, so - * concurrent edits elsewhere in settings are not overwritten. */ const updateMemoriesPreferences = async ( patch: UpdateUserPreferencesRequest['memories'], - ) => { - const previousPreferences = preferences; - setPreferences({ - ...preferences, - memories: { - ...preferences.memories, - ...patch, - weights: { ...preferences.memories.weights, ...patch?.weights }, + ) => + writePreferences((current) => ({ + next: { + ...current, + memories: { + ...current.memories, + ...patch, + weights: { ...current.memories.weights, ...patch?.weights }, + }, }, - }); - try { - return await updatePreferencesMutation.mutateAsync({ memories: patch }); - } catch (err) { - setPreferences(previousPreferences); - throw err; - } - }; + request: { memories: patch }, + })); return { // Data @@ -155,7 +178,6 @@ export const useUserPreferences = () => { isLoading: preferencesQuery.isLoading, // Operations - updatePreference, updateYoloModelSize, toggleGpuAcceleration, updateVideoFrameInterval, From fce96495c76cf83fb0c87e8de7f5322c6477eef7 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:35:42 +0530 Subject: [PATCH 6/9] test(settings): cover the memories panel Both directions of the min and max cross-clamp, a stored value that is not one of the dropdown options, and every control disabled while a save runs. --- .../__tests__/UserPreferencesCard.test.tsx | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 frontend/src/pages/SettingsPage/components/__tests__/UserPreferencesCard.test.tsx diff --git a/frontend/src/pages/SettingsPage/components/__tests__/UserPreferencesCard.test.tsx b/frontend/src/pages/SettingsPage/components/__tests__/UserPreferencesCard.test.tsx new file mode 100644 index 000000000..87799263b --- /dev/null +++ b/frontend/src/pages/SettingsPage/components/__tests__/UserPreferencesCard.test.tsx @@ -0,0 +1,188 @@ +import { render, screen } from '@/test-utils'; +import userEvent from '@testing-library/user-event'; + +import UserPreferencesCard from '../UserPreferencesCard'; +import type { MemoriesPreferences } from '@/api/api-functions/user_preferences'; + +const mockUpdateMemoriesPreferences = jest.fn().mockResolvedValue(undefined); +let mockMemories: MemoriesPreferences; +let mockIsUpdating = false; + +jest.mock('@/hooks/useUserPreferences', () => ({ + useUserPreferences: () => ({ + preferences: { + YOLO_model_size: 'nano', + GPU_Acceleration: false, + Video_Frame_Interval: 5, + memories: mockMemories, + }, + memoriesPreferences: mockMemories, + isLoading: false, + updateYoloModelSize: jest.fn().mockResolvedValue(undefined), + toggleGpuAcceleration: jest.fn().mockResolvedValue(undefined), + updateVideoFrameInterval: jest.fn().mockResolvedValue(undefined), + updateMemoriesPreferences: mockUpdateMemoriesPreferences, + refetch: jest.fn().mockResolvedValue(undefined), + isUpdating: mockIsUpdating, + }), +})); + +const memoriesWith = ( + overrides: Partial = {}, +): MemoriesPreferences => ({ + enabled: true, + notifications_enabled: false, + story_music_enabled: false, + slide_duration_seconds: 5, + min_images: 5, + max_images: 30, + weights: { + favourite: 0.22, + known_people: 0.2, + event_strength: 0.18, + face_presence: 0.12, + semantic_confidence: 0.1, + gps_novelty: 0.1, + in_album: 0.08, + }, + ...overrides, +}); + +/** Expand the collapsible group; its controls are not mounted until then. */ +const openPanel = async (user: ReturnType) => { + await user.click( + screen.getByRole('button', { name: /Control how memories are generated/i }), + ); +}; + +const trigger = (id: string) => + document.getElementById(id) as HTMLButtonElement; + +const choose = async ( + user: ReturnType, + id: string, + option: RegExp, +) => { + await user.click(trigger(id)); + await user.click(screen.getByRole('menuitem', { name: option })); +}; + +beforeEach(() => { + mockUpdateMemoriesPreferences.mockClear(); + mockMemories = memoriesWith(); + mockIsUpdating = false; +}); + +describe('UserPreferencesCard memories panel', () => { + it('sends the toggles as patches', async () => { + const user = userEvent.setup(); + render(); + await openPanel(user); + + await user.click( + screen.getByRole('switch', { name: /Generate Memories/i }), + ); + expect(mockUpdateMemoriesPreferences).toHaveBeenCalledWith({ + enabled: false, + }); + }); + + it('locks notifications behind the generate toggle', async () => { + mockMemories = memoriesWith({ enabled: false }); + const user = userEvent.setup(); + render(); + await openPanel(user); + + // Alerting about memories that are never generated is a dead control. + expect( + screen.getByRole('switch', { name: /Desktop Notifications/i }), + ).toBeDisabled(); + }); + + it('raises the maximum when a larger minimum is chosen', async () => { + // Reachable only from values the old sliders allowed; the dropdown + // options alone cannot put min above max. + mockMemories = memoriesWith({ min_images: 2, max_images: 5 }); + const user = userEvent.setup(); + render(); + await openPanel(user); + + await choose(user, 'memories-min', /^8 photos$/); + + expect(mockUpdateMemoriesPreferences).toHaveBeenCalledWith({ + min_images: 8, + max_images: 8, + }); + }); + + it('lowers the minimum when a smaller maximum is chosen', async () => { + mockMemories = memoriesWith({ min_images: 40, max_images: 50 }); + const user = userEvent.setup(); + render(); + await openPanel(user); + + await choose(user, 'memories-max', /^20 photos$/); + + expect(mockUpdateMemoriesPreferences).toHaveBeenCalledWith({ + max_images: 20, + min_images: 20, + }); + }); + + it('leaves a valid pair alone', async () => { + const user = userEvent.setup(); + render(); + await openPanel(user); + + await choose(user, 'memories-min', /^3 photos$/); + + expect(mockUpdateMemoriesPreferences).toHaveBeenCalledWith({ + min_images: 3, + max_images: 30, + }); + }); + + it('shows a stored value that is not one of the options', async () => { + mockMemories = memoriesWith({ + slide_duration_seconds: 12, + min_images: 40, + max_images: 50, + }); + const user = userEvent.setup(); + render(); + await openPanel(user); + + expect(trigger('memories-duration')).toHaveTextContent('12s'); + expect(trigger('memories-min')).toHaveTextContent('40 photos'); + expect(trigger('memories-max')).toHaveTextContent('50 photos'); + }); + + it('sends the chosen slide duration', async () => { + const user = userEvent.setup(); + render(); + await openPanel(user); + + await choose(user, 'memories-duration', /^7 seconds$/); + + expect(mockUpdateMemoriesPreferences).toHaveBeenCalledWith({ + slide_duration_seconds: 7, + }); + }); + + it('disables every control while a save is pending', async () => { + mockIsUpdating = true; + const user = userEvent.setup(); + render(); + await openPanel(user); + + expect( + screen.getByRole('switch', { name: /Generate Memories/i }), + ).toBeDisabled(); + expect( + screen.getByRole('switch', { name: /Desktop Notifications/i }), + ).toBeDisabled(); + expect(trigger('memories-duration')).toBeDisabled(); + expect(trigger('memories-min')).toBeDisabled(); + expect(trigger('memories-max')).toBeDisabled(); + }); +}); From 3e4eb2e1887c3a00f062af6fd1b66c5732c1bd34 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:57:59 +0530 Subject: [PATCH 7/9] Remove accessibility to settings via memories page --- frontend/src/pages/Memories/Memories.tsx | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/frontend/src/pages/Memories/Memories.tsx b/frontend/src/pages/Memories/Memories.tsx index 4b2cd6119..3d70802d0 100644 --- a/frontend/src/pages/Memories/Memories.tsx +++ b/frontend/src/pages/Memories/Memories.tsx @@ -1,11 +1,9 @@ import React, { useEffect } from 'react'; -import { RefreshCw, Settings } from 'lucide-react'; -import { useNavigate } from 'react-router'; +import { RefreshCw } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { MemoryCard } from '@/components/Memories/MemoryCard'; import { MemoryStoryViewer } from '@/components/Memories/MemoryStoryViewer'; -import { ROUTES } from '@/constants/routes'; import { showInfoDialog } from '@/features/infoDialogSlice'; import { useAppDispatch, useAppSelector } from '@/store/hooks'; import { useUserPreferences } from '@/hooks/useUserPreferences'; @@ -33,7 +31,6 @@ const EmptyState: React.FC<{ isGenerating: boolean }> = ({ isGenerating }) => ( export const Memories: React.FC = () => { const dispatch = useAppDispatch(); - const navigate = useNavigate(); const activeMemoryId = useAppSelector(selectActiveMemoryId); const memoriesQuery = useMemories({ limit: 60 }); @@ -103,15 +100,7 @@ export const Memories: React.FC = () => { - {isGenerating ? 'Generating…' : 'Refresh'} - -
From e6e81c724738f79de1137f3f97ac9ee3946ac7f9 Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:45:40 +0530 Subject: [PATCH 8/9] fix(settings): stop a stale load overwriting a queued write The mutation refetched on success, outside the queue, so a response that predated a newer write could land on top of it and hand the next queued write a stale base. The PUT already returns the merged result, so the write path adopts that instead, and the load effect stands down while writes are pending. Tests drop the never-resolving read they used to need. --- .../__tests__/useUserPreferences.test.tsx | 108 +++++++++++++++--- frontend/src/hooks/useUserPreferences.tsx | 29 ++++- 2 files changed, 116 insertions(+), 21 deletions(-) diff --git a/frontend/src/hooks/__tests__/useUserPreferences.test.tsx b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx index f9a167742..069a7d824 100644 --- a/frontend/src/hooks/__tests__/useUserPreferences.test.tsx +++ b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx @@ -72,23 +72,41 @@ const mountLoaded = async () => { const sentBodies = () => mockUpdateUserPreferences.mock.calls.map(([body]) => body); +/** Mirrors the route's _deep_merge, so a patch behaves as the server does. */ +const deepMerge = (base: any, updates: any): any => { + const merged = { ...base }; + for (const [key, value] of Object.entries(updates ?? {})) { + const isPlainObject = (v: unknown) => + typeof v === 'object' && v !== null && !Array.isArray(v); + merged[key] = + isPlainObject(value) && isPlainObject(merged[key]) + ? deepMerge(merged[key], value) + : value; + } + return merged; +}; + +/** Stands in for the stored blob so reads and writes agree. */ +let stored: UserPreferencesData; +const payload = (user_preferences: UserPreferencesData) => ({ + success: true, + message: 'ok', + user_preferences, +}); + beforeEach(() => { + stored = JSON.parse(JSON.stringify(baseline)); + mockGetUserPreferences .mockReset() - // The refetch after a successful write would otherwise overwrite local - // state from the server and mask what optimistic apply and rollback did. - .mockResolvedValueOnce({ - success: true, - message: 'ok', - user_preferences: baseline, - }) - .mockImplementation(() => new Promise(() => {})); - - mockUpdateUserPreferences.mockReset().mockResolvedValue({ - success: true, - message: 'ok', - user_preferences: baseline, - }); + .mockImplementation(async () => payload(stored)); + + mockUpdateUserPreferences + .mockReset() + .mockImplementation(async (request: UpdateUserPreferencesRequest) => { + stored = deepMerge(stored, request); + return payload(stored); + }); }); describe('useUserPreferences', () => { @@ -128,7 +146,8 @@ describe('useUserPreferences', () => { if (request.Video_Frame_Interval !== undefined) { throw new Error('save failed'); } - return { success: true, message: 'ok', user_preferences: baseline }; + stored = deepMerge(stored, request); + return payload(stored); }, ); @@ -170,6 +189,65 @@ describe('useUserPreferences', () => { expect(result.current.preferences.Video_Frame_Interval).toBe(10); }); + it('ignores a load that lands while a write is pending', async () => { + const result = await mountLoaded(); + + // A read already in flight when the write starts. It carries someone + // else's change, so react-query does not dedupe it, and the pre-write + // value for the key the write is about to change. + mockGetUserPreferences.mockResolvedValue( + payload({ + ...JSON.parse(JSON.stringify(baseline)), + YOLO_model_size: 'small', + Video_Frame_Interval: 5, + }), + ); + + let release = () => {}; + mockUpdateUserPreferences.mockImplementationOnce( + async (request: UpdateUserPreferencesRequest) => { + await new Promise((resolve) => { + release = resolve; + }); + stored = deepMerge(stored, request); + return payload(stored); + }, + ); + + let write: Promise = Promise.resolve(); + await act(async () => { + write = result.current.updateVideoFrameInterval(30); + await result.current.refetch(); + }); + + // Applying the load here would revert the optimistic value and hand the + // next queued write a stale base. + expect(result.current.preferences.Video_Frame_Interval).toBe(30); + + await act(async () => { + release(); + await write; + }); + + expect(result.current.preferences.Video_Frame_Interval).toBe(30); + }); + + it('adopts the merged result the server returns', async () => { + const result = await mountLoaded(); + + // The server normalizes weights on read, so the response is the truth + // rather than what was optimistically applied. + mockUpdateUserPreferences.mockImplementationOnce(async () => + payload(deepMerge(stored, { Video_Frame_Interval: 2 })), + ); + + await act(async () => { + await result.current.updateVideoFrameInterval(30); + }); + + expect(result.current.preferences.Video_Frame_Interval).toBe(2); + }); + it('merges a memories patch over the stored weights', async () => { const result = await mountLoaded(); diff --git a/frontend/src/hooks/useUserPreferences.tsx b/frontend/src/hooks/useUserPreferences.tsx index 61563fa4c..ec1210814 100644 --- a/frontend/src/hooks/useUserPreferences.tsx +++ b/frontend/src/hooks/useUserPreferences.tsx @@ -54,8 +54,15 @@ export const useUserPreferences = () => { queryFn: getUserPreferences, }); + // Non-zero from the moment a write is queued until it settles. A load that + // was already in flight carries server state from before that write, so + // applying it would revert the change and hand the next queued write a stale + // base to build on. + const pendingWrites = useRef(0); + // Update local state when preferences data changes useEffect(() => { + if (pendingWrites.current > 0) return; if ( preferencesQuery.data?.success && preferencesQuery.data.user_preferences @@ -65,13 +72,11 @@ export const useUserPreferences = () => { // eslint-disable-next-line react-hooks/exhaustive-deps }, [preferencesQuery.data]); - // Mutation for updating user preferences + // Mutation for updating user preferences. It does not refetch on success: + // the response already carries the merged result, and reconciling inside the + // queue is what keeps a stale read from overtaking a newer write. const updatePreferencesMutation = usePictoMutation({ mutationFn: updateUserPreferences, - onSuccess: () => { - // Invalidate and refetch preferences - preferencesQuery.refetch(); - }, }); // Apply feedback to the update preferences mutation but hide loader and success dialog @@ -105,15 +110,27 @@ export const useUserPreferences = () => { request: UpdateUserPreferencesRequest; }, ) => { + // Counted here rather than in `send` so a load cannot slip in between the + // click and the write reaching the front of the queue. + pendingWrites.current += 1; + const send = async () => { const current = preferencesRef.current; const { next, request } = build(current); applyPreferences(next); try { - return await updatePreferencesMutation.mutateAsync(request); + const response = await updatePreferencesMutation.mutateAsync(request); + // The PUT returns the merged, validated result. Adopting it here keeps + // reconciliation inside the queue, where nothing else is in flight. + if (response?.success && response.user_preferences) { + applyPreferences(response.user_preferences); + } + return response; } catch (err) { applyPreferences(current); throw err; + } finally { + pendingWrites.current -= 1; } }; From 83bd5577a0da0c9bbcaf7315838f8317bd02e92b Mon Sep 17 00:00:00 2001 From: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:58:27 +0530 Subject: [PATCH 9/9] fix(settings): reject a load that started before a queued write The pending-write guard only covered reads that arrived mid-write. A read issued earlier can resolve after the write settles, and it carries pre-write state, so it reverted the change. Reads now capture a write counter when they start and are dropped if a write has been queued since. --- .../__tests__/useUserPreferences.test.tsx | 72 ++++++++++++++++--- frontend/src/hooks/useUserPreferences.tsx | 24 +++++-- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/frontend/src/hooks/__tests__/useUserPreferences.test.tsx b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx index 069a7d824..43832b18c 100644 --- a/frontend/src/hooks/__tests__/useUserPreferences.test.tsx +++ b/frontend/src/hooks/__tests__/useUserPreferences.test.tsx @@ -72,18 +72,33 @@ const mountLoaded = async () => { const sentBodies = () => mockUpdateUserPreferences.mock.calls.map(([body]) => body); +/** + * Let the query effect run. + * + * A response reaches the cache a tick before the effect that would apply it, + * so asserting straight after the request resolves reads state that has not + * been overwritten yet, whether or not anything guards it. + */ +const settle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +}; + +const isPlainObject = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + /** Mirrors the route's _deep_merge, so a patch behaves as the server does. */ -const deepMerge = (base: any, updates: any): any => { - const merged = { ...base }; - for (const [key, value] of Object.entries(updates ?? {})) { - const isPlainObject = (v: unknown) => - typeof v === 'object' && v !== null && !Array.isArray(v); +const deepMerge = (base: T, updates: object): T => { + const merged = { ...base } as Record; + for (const [key, value] of Object.entries(updates)) { + const existing = merged[key]; merged[key] = - isPlainObject(value) && isPlainObject(merged[key]) - ? deepMerge(merged[key], value) + isPlainObject(value) && isPlainObject(existing) + ? deepMerge(existing, value) : value; } - return merged; + return merged as T; }; /** Stands in for the stored blob so reads and writes agree. */ @@ -228,8 +243,49 @@ describe('useUserPreferences', () => { release(); await write; }); + await settle(); + + expect(result.current.preferences.Video_Frame_Interval).toBe(30); + }); + + it('ignores a load that started before a write but lands after it', async () => { + const result = await mountLoaded(); + + // Held open until the write has fully settled, so the pending-write guard + // is back to zero by the time the response arrives. + let releaseRead = () => {}; + mockGetUserPreferences.mockImplementationOnce(async () => { + await new Promise((resolve) => { + releaseRead = resolve; + }); + return payload({ + ...JSON.parse(JSON.stringify(baseline)), + YOLO_model_size: 'small', + Video_Frame_Interval: 5, + }); + }); + + let read: Promise = Promise.resolve(); + await act(async () => { + read = result.current.refetch(); + await Promise.resolve(); + }); + + await act(async () => { + await result.current.updateVideoFrameInterval(30); + }); + expect(result.current.preferences.Video_Frame_Interval).toBe(30); + + await act(async () => { + releaseRead(); + await read; + }); + await settle(); + // A read that starts first can still finish last, so arrival order alone + // cannot tell this response apart from a current one. expect(result.current.preferences.Video_Frame_Interval).toBe(30); + expect(result.current.preferences.YOLO_model_size).toBe('medium'); }); it('adopts the merged result the server returns', async () => { diff --git a/frontend/src/hooks/useUserPreferences.tsx b/frontend/src/hooks/useUserPreferences.tsx index ec1210814..8a6aedad5 100644 --- a/frontend/src/hooks/useUserPreferences.tsx +++ b/frontend/src/hooks/useUserPreferences.tsx @@ -48,21 +48,30 @@ export const useUserPreferences = () => { setPreferences(next); }; + // Non-zero from the moment a write is queued until it settles. + const pendingWrites = useRef(0); + // Bumped when a write is queued. Any read already in flight at that point + // describes the server from before it, however late the response arrives, + // which is why this counts reads rather than timing them: a read that starts + // first can still finish last. + const writeEpoch = useRef(0); + const readEpoch = useRef(0); + // Query for user preferences const preferencesQuery = usePictoQuery({ queryKey: ['userPreferences'], - queryFn: getUserPreferences, + queryFn: () => { + readEpoch.current = writeEpoch.current; + return getUserPreferences(); + }, }); - // Non-zero from the moment a write is queued until it settles. A load that - // was already in flight carries server state from before that write, so - // applying it would revert the change and hand the next queued write a stale - // base to build on. - const pendingWrites = useRef(0); - // Update local state when preferences data changes useEffect(() => { + // Applying stale server state would revert the write and hand the next + // queued one a stale base to build on. if (pendingWrites.current > 0) return; + if (readEpoch.current !== writeEpoch.current) return; if ( preferencesQuery.data?.success && preferencesQuery.data.user_preferences @@ -113,6 +122,7 @@ export const useUserPreferences = () => { // Counted here rather than in `send` so a load cannot slip in between the // click and the write reaching the front of the queue. pendingWrites.current += 1; + writeEpoch.current += 1; const send = async () => { const current = preferencesRef.current;