Skip to content

Commit d42139e

Browse files
pythoninthegrassclaude
pythoninthegrass
and
claude
authored
feat(library): unified library_get_section command (#40)
* feat(library): add unified library_get_section command Replace separate library_get_count + library_get_all calls with a single library_get_section Tauri command that returns tracks + authoritative stats (total_tracks, total_duration) in one SQLite transaction. - Add library_revision table with monotonic counter for cache invalidation - Add stats query functions for favorites, top25, recent, added, playlists - Bump revision on all library mutation paths (add/delete/favorite/playlist) - Add getSection() API method on frontend - Simplify store section loaders to use unified endpoint - Prefer backend-owned total_tracks/total_duration over JS computation - Add 14 Rust tests and 10 Vitest tests for the new command Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): remove unused imports and update FOUC tests for unified endpoint - Remove unused favorites and playlists imports from library store (unified endpoint handles all sections via library.getSection) - Update FOUC regression tests to mock library.getSection instead of library.getCount + store._fetchPage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): prefix unused conn variable in test_section_unknown Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): add HTTP fallback for getSection in Playwright E2E tests The getSection method only had a Tauri invoke path and threw a 501 in non-Tauri environments. Playwright E2E tests run without a Tauri backend, so all accessibility tests timed out waiting for tracks to load. Add an HTTP fallback that composes existing /library + /library/count endpoints into the unified response shape. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): handle playlist sections in getSection HTTP fallback The HTTP fallback for getSection always called /library + /library/count regardless of section type. Playlist sections (playlist-{id}) need to call /playlists/:id instead, matching the pre-existing REST endpoint that E2E mock fixtures provide. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(e2e): add HTTP fallback for queue.playContext() Replace the hard throw in browser mode with a REST POST to /queue/play-context. Add corresponding Playwright route mock that builds queue items from library state, rotates to the start track, and returns the expected response shape. Fixes 10 pre-existing E2E failures where double-click-to-play timed out because playContext was unavailable without Tauri IPC. * fix(e2e): route missing tracks through playTrack for modal display handleDoubleClickPlay bypassed player.playTrack() (which contains the missing track modal check) by going directly to playContext. Add track.missing to the guard clause so missing tracks fall through to playTrack, which shows the missing track modal as expected. * fix: skip _filterByLibrary for unified backend section loads When using the unified library_get_section backend command, the returned tracks are already authoritative for the requested section. Filtering them against the 'all' section's filteredTracks (via _filterByLibrary) incorrectly drops all tracks when the 'all' section hasn't been loaded or only has a partial first page. Skip the filter in both loadSection and backgroundRefreshSection when useUnified is true. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c900d0c commit d42139e

File tree

17 files changed

+1397
-99
lines changed

17 files changed

+1397
-99
lines changed

‎app/frontend/__tests__/library-section.test.js‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* Tests for unified library_get_section integration.
3+
*
4+
* Verifies that applySectionData and buildCacheEntry correctly use
5+
* authoritative stats from the backend response (total_tracks, total_duration)
6+
* instead of computing them from the tracks array.
7+
*/
8+
9+
import { describe, expect, it } from 'vitest';
10+
11+
// Mock window and Alpine before importing modules that depend on them
12+
globalThis.window = {
13+
...globalThis.window,
14+
Alpine: {
15+
disableEffectScheduling: (fn) => fn(),
16+
},
17+
__TAURI__: undefined,
18+
};
19+
20+
// Dynamic import to ensure window mock is set up first
21+
const { applySectionData } = await import('../js/utils/library-operations.js');
22+
const { buildCacheEntry } = await import('../js/utils/library-cache.js');
23+
24+
// ---------------------------------------------------------------------------
25+
// applySectionData
26+
// ---------------------------------------------------------------------------
27+
28+
describe('applySectionData', () => {
29+
function createMockStore() {
30+
return {
31+
totalTracks: 0,
32+
totalDuration: 0,
33+
_lastLoadedSection: null,
34+
_sectionTracks: null,
35+
_setSectionTracks(tracks) {
36+
this._sectionTracks = tracks;
37+
},
38+
};
39+
}
40+
41+
it('uses total_tracks and total_duration from backend response', () => {
42+
const store = createMockStore();
43+
const tracks = [
44+
{ id: 1, duration: 100 },
45+
{ id: 2, duration: 200 },
46+
];
47+
const data = {
48+
total_tracks: 42,
49+
total_duration: 12345.5,
50+
};
51+
52+
applySectionData(store, 'all', tracks, data);
53+
54+
expect(store.totalTracks).toBe(42);
55+
expect(store.totalDuration).toBe(12345.5);
56+
expect(store._lastLoadedSection).toBe('all');
57+
expect(store._sectionTracks).toEqual(tracks);
58+
});
59+
60+
it('falls back to total field when total_tracks is absent', () => {
61+
const store = createMockStore();
62+
const tracks = [{ id: 1, duration: 60 }];
63+
const data = { total: 10 };
64+
65+
applySectionData(store, 'liked', tracks, data);
66+
67+
expect(store.totalTracks).toBe(10);
68+
});
69+
70+
it('falls back to tracks.length when no total fields present', () => {
71+
const store = createMockStore();
72+
const tracks = [{ id: 1 }, { id: 2 }, { id: 3 }];
73+
74+
applySectionData(store, 'recent', tracks, {});
75+
76+
expect(store.totalTracks).toBe(3);
77+
});
78+
79+
it('falls back to computing totalDuration from tracks when total_duration absent', () => {
80+
const store = createMockStore();
81+
const tracks = [
82+
{ id: 1, duration: 100 },
83+
{ id: 2, duration: 200 },
84+
];
85+
86+
applySectionData(store, 'added', tracks, {});
87+
88+
expect(store.totalDuration).toBe(300);
89+
});
90+
91+
it('prefers total_duration of 0 over JS-computed fallback', () => {
92+
const store = createMockStore();
93+
const tracks = [{ id: 1, duration: 100 }];
94+
const data = { total_duration: 0 };
95+
96+
applySectionData(store, 'top25', tracks, data);
97+
98+
// total_duration: 0 is a valid authoritative value
99+
expect(store.totalDuration).toBe(0);
100+
});
101+
});
102+
103+
// ---------------------------------------------------------------------------
104+
// buildCacheEntry
105+
// ---------------------------------------------------------------------------
106+
107+
describe('buildCacheEntry', () => {
108+
it('uses total_tracks and total_duration from backend response', () => {
109+
const data = {
110+
tracks: [{ duration: 100 }],
111+
total_tracks: 500,
112+
total_duration: 99999.0,
113+
};
114+
115+
const entry = buildCacheEntry(data);
116+
117+
expect(entry.totalTracks).toBe(500);
118+
expect(entry.totalDuration).toBe(99999.0);
119+
expect(entry.timestamp).toBeGreaterThan(0);
120+
});
121+
122+
it('falls back to total field when total_tracks is absent', () => {
123+
const data = {
124+
tracks: [{ duration: 100 }],
125+
total: 10,
126+
};
127+
128+
const entry = buildCacheEntry(data);
129+
130+
expect(entry.totalTracks).toBe(10);
131+
});
132+
133+
it('falls back to tracks.length when no total fields', () => {
134+
const data = {
135+
tracks: [{ duration: 100 }, { duration: 200 }],
136+
};
137+
138+
const entry = buildCacheEntry(data);
139+
140+
expect(entry.totalTracks).toBe(2);
141+
});
142+
143+
it('falls back to computing duration from tracks when total_duration absent', () => {
144+
const data = {
145+
tracks: [{ duration: 100 }, { duration: 200 }],
146+
};
147+
148+
const entry = buildCacheEntry(data);
149+
150+
expect(entry.totalDuration).toBe(300);
151+
});
152+
153+
it('handles empty tracks with backend stats', () => {
154+
const data = {
155+
tracks: [],
156+
total_tracks: 0,
157+
total_duration: 0,
158+
};
159+
160+
const entry = buildCacheEntry(data);
161+
162+
expect(entry.totalTracks).toBe(0);
163+
expect(entry.totalDuration).toBe(0);
164+
});
165+
});

‎app/frontend/__tests__/library.store.test.js‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -569,13 +569,23 @@ describe('Paginated Store - _isPaginated', () => {
569569
describe('loadLibraryData - FOUC regression', () => {
570570
let loadLibraryData;
571571

572+
let mockGetSection;
573+
572574
beforeEach(async () => {
573575
vi.resetModules();
574576

575-
// Mock the library API
577+
// Mock the library API with getSection (unified endpoint)
578+
mockGetSection = vi.fn().mockResolvedValue({
579+
section: 'all',
580+
tracks: makeTracks(50),
581+
total_tracks: 50,
582+
total_duration: 3600,
583+
has_more: false,
584+
revision: 1,
585+
});
576586
vi.doMock('../js/api/library.js', () => ({
577587
library: {
578-
getCount: vi.fn().mockResolvedValue({ total: 50, total_duration: 3600 }),
588+
getSection: mockGetSection,
579589
},
580590
}));
581591

@@ -603,7 +613,6 @@ describe('loadLibraryData - FOUC regression', () => {
603613
store.currentSection = 'all';
604614
store._lastLoadedSection = 'all';
605615
store._sectionCache = {};
606-
store._fetchPage = vi.fn().mockResolvedValue(undefined);
607616
store._getFilterParams = () => ({});
608617
store._updateCache = vi.fn();
609618
return store;
@@ -613,14 +622,21 @@ describe('loadLibraryData - FOUC regression', () => {
613622
const store = createLoadStore();
614623
const statesDuringFetch = [];
615624

616-
// Capture state when _fetchPage is called (during the await)
617-
store._fetchPage = vi.fn().mockImplementation(() => {
625+
// Capture state when getSection is called (during the await)
626+
mockGetSection.mockImplementation(() => {
618627
statesDuringFetch.push({
619628
totalTracks: store.totalTracks,
620629
loading: store.loading,
621630
pagesEmpty: Object.keys(store._trackPages).length === 0,
622631
});
623-
return Promise.resolve();
632+
return Promise.resolve({
633+
section: 'all',
634+
tracks: makeTracks(50),
635+
total_tracks: 50,
636+
total_duration: 3600,
637+
has_more: false,
638+
revision: 1,
639+
});
624640
});
625641

626642
await loadLibraryData(store);
@@ -644,9 +660,16 @@ describe('loadLibraryData - FOUC regression', () => {
644660
};
645661

646662
const tracksSeenDuringFetch = [];
647-
store._fetchPage = vi.fn().mockImplementation(() => {
663+
mockGetSection.mockImplementation(() => {
648664
tracksSeenDuringFetch.push(store.totalTracks);
649-
return Promise.resolve();
665+
return Promise.resolve({
666+
section: 'all',
667+
tracks: makeTracks(50),
668+
total_tracks: 50,
669+
total_duration: 3600,
670+
has_more: false,
671+
revision: 1,
672+
});
650673
});
651674

652675
await loadLibraryData(store);

‎app/frontend/js/api/library.js‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,90 @@ export const library = {
103103
return request(`/library${queryString ? `?${queryString}` : ''}`);
104104
},
105105

106+
/**
107+
* Get a complete view model for any library section in a single call.
108+
* Returns tracks, authoritative stats, pagination metadata, and a revision
109+
* number for cache invalidation — all from the same DB transaction.
110+
* @param {object} params
111+
* @param {string} params.section - 'all', 'liked', 'top25', 'recent', 'added', or 'playlist-{id}'
112+
* @param {string} [params.search] - Search query (all section only)
113+
* @param {string} [params.artist] - Artist filter (all section only)
114+
* @param {string} [params.album] - Album filter (all section only)
115+
* @param {string} [params.sort] - Sort field (all section only)
116+
* @param {string} [params.order] - Sort order (all section only)
117+
* @param {number} [params.limit] - Page size / result limit
118+
* @param {number} [params.offset] - Page offset (all section only)
119+
* @param {string} [params.ignoreWords] - Ignore words for sort (all section only)
120+
* @param {number} [params.days] - Days lookback (recent/added sections)
121+
* @returns {Promise<{section: string, tracks: Array, total_tracks: number, total_duration: number, page: number|null, page_size: number|null, has_more: boolean, revision: number}>}
122+
*/
123+
async getSection(params = {}) {
124+
if (invoke) {
125+
try {
126+
return await invoke('library_get_section', {
127+
section: params.section,
128+
search: params.search || null,
129+
artist: params.artist || null,
130+
album: params.album || null,
131+
sortBy: params.sort || null,
132+
sortOrder: params.order || null,
133+
limit: params.limit || null,
134+
offset: params.offset || null,
135+
ignoreWords: params.ignoreWords || null,
136+
days: params.days || null,
137+
});
138+
} catch (error) {
139+
console.error('[api.library.getSection] Tauri error:', error);
140+
throw new ApiError(500, error.toString());
141+
}
142+
}
143+
// HTTP fallback: dispatch to the appropriate REST endpoint per section
144+
const section = params.section || 'all';
145+
146+
// Playlist sections use the playlists REST endpoint
147+
const playlistMatch = section.match(/^playlist-(\d+)$/);
148+
if (playlistMatch) {
149+
const data = await request(`/playlists/${playlistMatch[1]}`);
150+
const tracks = (data.tracks || []).map((item) => item.track || item);
151+
const totalDuration = tracks.reduce((sum, t) => sum + (t.duration || 0), 0);
152+
return {
153+
section,
154+
tracks,
155+
total_tracks: tracks.length,
156+
total_duration: totalDuration,
157+
page: null,
158+
page_size: null,
159+
has_more: false,
160+
revision: 0,
161+
};
162+
}
163+
164+
// All other sections compose from /library + /library/count
165+
const query = new URLSearchParams();
166+
if (params.search) query.set('search', params.search);
167+
if (params.artist) query.set('artist', params.artist);
168+
if (params.album) query.set('album', params.album);
169+
if (params.sort) query.set('sort_by', params.sort);
170+
if (params.order) query.set('sort_order', params.order);
171+
if (params.limit) query.set('limit', params.limit.toString());
172+
if (params.offset) query.set('offset', params.offset.toString());
173+
const qs = query.toString();
174+
const [trackData, countData] = await Promise.all([
175+
request(`/library${qs ? `?${qs}` : ''}`),
176+
request(`/library/count${qs ? `?${qs}` : ''}`),
177+
]);
178+
return {
179+
section,
180+
tracks: trackData.tracks || [],
181+
total_tracks: countData.total ?? (trackData.tracks || []).length,
182+
total_duration: countData.total_duration ?? 0,
183+
page: params.offset != null ? Math.floor(params.offset / (params.limit || 50)) : null,
184+
page_size: params.limit || null,
185+
has_more: trackData.total > (params.offset || 0) + (trackData.tracks || []).length,
186+
revision: 0,
187+
};
188+
},
189+
106190
/**
107191
* Get a single track by ID (uses Tauri command)
108192
* @param {number} id - Track ID

‎app/frontend/js/api/queue.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,14 @@ export const queue = {
175175
throw new ApiError(500, error.toString());
176176
}
177177
}
178-
throw new ApiError(500, 'playContext not available in browser mode');
178+
return request('/queue/play-context', {
179+
method: 'POST',
180+
body: JSON.stringify({
181+
track_ids: trackIds,
182+
start_index: startIndex,
183+
shuffle,
184+
}),
185+
});
179186
},
180187

181188
save(state) {

0 commit comments

Comments
 (0)