diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..b4e9a99 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +# The historical source revision contains one NUL separator; keep its repair reviewable. +src/db/queries/scanning.ts diff diff --git a/app/(tabs)/_layout.tsx b/app/(tabs)/_layout.tsx index f95ca72..50441c2 100644 --- a/app/(tabs)/_layout.tsx +++ b/app/(tabs)/_layout.tsx @@ -10,6 +10,7 @@ import { View } from 'react-native'; import { Toaster } from '@/components/ui/Toaster'; import { MiniPlayer } from '@/features/player/components/MiniPlayer'; +import * as perf from '@/services/perf'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { useTheme } from '@/theme/useTheme'; @@ -57,6 +58,10 @@ export default function TabsLayout() { return ( ({ + tabPress: () => perf.mark(`tab.${route.name}.focus`), + focus: () => perf.measure(`tab.${route.name}.focus`), + })} screenOptions={{ headerShown: false, // Indigo marks the active tab and nothing else in this bar. diff --git a/app/(tabs)/index.tsx b/app/(tabs)/index.tsx index c981317..30a0f23 100644 --- a/app/(tabs)/index.tsx +++ b/app/(tabs)/index.tsx @@ -1,3 +1,10 @@ +import { TabErrorBoundary } from '@/components/ui/TabErrorBoundary'; import { LibraryScreen } from '@/features/library/LibraryScreen'; -export default LibraryScreen; +export default function LibraryRoute() { + return ( + + + + ); +} diff --git a/app/(tabs)/playlists.tsx b/app/(tabs)/playlists.tsx index 5d7f881..722a169 100644 --- a/app/(tabs)/playlists.tsx +++ b/app/(tabs)/playlists.tsx @@ -1,3 +1,10 @@ +import { TabErrorBoundary } from '@/components/ui/TabErrorBoundary'; import { PlaylistsScreen } from '@/features/playlists/PlaylistsScreen'; -export default PlaylistsScreen; +export default function PlaylistsRoute() { + return ( + + + + ); +} diff --git a/app/(tabs)/settings.tsx b/app/(tabs)/settings.tsx index 0437d25..b8fcb5a 100644 --- a/app/(tabs)/settings.tsx +++ b/app/(tabs)/settings.tsx @@ -1,3 +1,10 @@ +import { TabErrorBoundary } from '@/components/ui/TabErrorBoundary'; import { SettingsScreen } from '@/features/settings/SettingsScreen'; -export default SettingsScreen; +export default function SettingsRoute() { + return ( + + + + ); +} diff --git a/app/(tabs)/stats.tsx b/app/(tabs)/stats.tsx index 2a1402f..daf3158 100644 --- a/app/(tabs)/stats.tsx +++ b/app/(tabs)/stats.tsx @@ -1,3 +1,10 @@ +import { TabErrorBoundary } from '@/components/ui/TabErrorBoundary'; import { StatsScreen } from '@/features/stats/StatsScreen'; -export default StatsScreen; +export default function StatsRoute() { + return ( + + + + ); +} diff --git a/docs/adr/012-artist-and-album-shelves.md b/docs/adr/012-artist-and-album-shelves.md index 70c907f..f479163 100644 --- a/docs/adr/012-artist-and-album-shelves.md +++ b/docs/adr/012-artist-and-album-shelves.md @@ -4,9 +4,9 @@ The library had one face: an alphabetical list of every track. The tech stack doc calls for "Tracks / Albums / Artists / Genres" segments, and until now only -the first existed. Nothing in the schema was missing — `artists` and `albums` -have been populated by the scanner since Phase 2 — so this was a query and a -screen, not a data problem. +the first existed. The schema already had `artists` and `albums`; scanner writes +now resolve their foreign keys from MediaStore and stage-two tags, so the shelf +remains a query and screen concern rather than duplicated metadata. ## Decision @@ -59,10 +59,10 @@ bounded by the result limit; the card queries use `min(artwork_path)` over the group, which is arbitrary but stable — and stable matters more than which, because a card whose cover changes between renders looks broken. -Both card queries `INNER JOIN` tracks rather than starting from `artists`. The -artist table gains a row the first time a name is seen and nothing ever removes -one, so a left join would list artists with zero tracks. What is on the device is -what the library shows. +Both card queries start at present `tracks`, so an artist row left behind by a +removed album cannot show as an empty shelf. They left-join their normalised +table and group null metadata into one translated unknown card; what is on the +device is what the library shows. ## Postscript: a class that compiled to nothing diff --git a/docs/adr/013-unknown-metadata.md b/docs/adr/013-unknown-metadata.md new file mode 100644 index 0000000..765e84d --- /dev/null +++ b/docs/adr/013-unknown-metadata.md @@ -0,0 +1,23 @@ +# 013 — Unknown metadata stays null + +## Context + +Some audio files have no artist or album tag. Treating that as a string made +one language leak into persistent data and left null-safe failures in grouping, +sorting and the artist screen. Skipping null artist and album rollups made the +statistics omit real listening time. + +## Decision + +Tracks keep `artist_id` and `album_id` null when metadata is absent. Library +queries group those nulls as one reserved display card, while rollups use entity +id `0`, which cannot collide with SQLite's positive ids. UI labels that entry +through i18n as Unknown Artist or Unknown Album; scanner maps `ARTIST`, +`ALBUM`, and `ALBUMARTIST` into the normalised foreign keys when they exist. + +## Consequences + +Missing metadata is visible, navigable and counted without storing a translated +placeholder row. Stats queries must left-join artists and albums for id `0`. +Balanced shuffle continues to treat each null artist as an individual bucket, +which avoids inventing a single performer for unrelated loose tracks. diff --git a/docs/components.md b/docs/components.md index 837d915..12aa94f 100644 --- a/docs/components.md +++ b/docs/components.md @@ -31,6 +31,7 @@ it is repeated here because that is the thing a reader needs before touching it. | `Screen` | The standard frame: safe area, surface, display-face title. | | `EmptyState` | Icon, one line, and the way out. Picks one of several phrasings per mount so the app does not read like a recording. | | `ErrorState` | What failed in one plain sentence, and the retry. Never a raw error string. | +| `TabErrorBoundary` | Contains a render failure to its selected tab and shows `ErrorState`; retry remounts only that tab. | | `Skeleton` | One placeholder block, sized by the caller. Pulses via a Reanimated worklet; stops dead under reduce-motion. | | `SkeletonRows` | A list's worth, shaped like real rows so nothing jumps when data lands. Hidden from screen readers. | | `SkeletonCards` | The same for the artist and album grids. Mirrors `CollectionGrid`'s two-column layout exactly. | @@ -73,7 +74,7 @@ it is repeated here because that is the thing a reader needs before touching it. |---|---| | `PlayerScreen` | Now Playing. A modal route: somewhere you go from something and dismiss. | | `ArtworkCarousel` | Three slots — previous, current, next — all mounted, so a neighbour slides in already decoded. Commits on distance **or** velocity. Rubber-bands at the ends of the queue. | -| `MiniPlayer` | The persistent transport strip. Subscribes to phase and track only, never position: doing otherwise reconciled it 20 times per ten seconds of playback. | +| `MiniPlayer` | The persistent transport strip. Subscribes to phase and track only, never position; horizontal swipe changes tracks and vertical swipe opens Now Playing. | | `MiniProgress` | The progress hairline, and the only thing in the tab bar that hears about position. Width lives in a shared value; React renders it once. | | `Scrubber` | The seek bar. The drag runs entirely in a worklet; React hears about it once, on release. | | `SpecStrip` | The signature element — one monospaced line of a file's technical truth. | diff --git a/docs/performance.md b/docs/performance.md index e54c499..327d424 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -128,6 +128,22 @@ Twenty is exactly the 2 Hz status interval. Fixed by two narrow subscriptions nothing changed, plus moving position into `MiniProgress` — one animated view whose width lives in a Reanimated shared value and which React renders once. +### Row press + +The library row previously mapped every track to a playable queue and searched +that same array inside its press handler. `MUFIFY_PERF` measured one press on +the same 10,000-track synthetic library and the same first row on the Pixel_7 +AVD debug build: + +| | Before | After | +|---|---:|---:| +| Press handler | **16.6 ms** | **0.8 ms** | +| Press to mini-player state | **168.5 ms** | **37.4 ms** | + +The queue and id-to-index map now update when the query result changes, not +when a row is pressed. The second number ends when the mini-player observes the +new current track; it is UI feedback latency, not time to audible audio. + **Worth recording what this was not.** The first diagnosis was that the tab bar next door was re-rendering with it. The counter said 0, both before and after: React re-renders the component whose store changed and its children, not its @@ -223,7 +239,7 @@ never been seen by anyone. `src/theme/scale.test.ts` now fails on any such class ## Regression pass Run after the three critical sections closed, on the Pixel_7 AVD, with both -gates green — `lint`, `typecheck`, 292 JS tests across 20 suites, and +gates green — `lint`, `typecheck`, 302 JS tests across 21 suites, and `:audio-tags:testDebugUnitTest` forced with `--rerun-tasks`. | Area | Result | diff --git a/docs/scanner.md b/docs/scanner.md index dc4b2ad..3a08a2b 100644 --- a/docs/scanner.md +++ b/docs/scanner.md @@ -94,7 +94,10 @@ usable within a second or two of a cold scan. ### Stage two — enrich `enrichLibrary()`. Opens files in batches of 25 and fills in tags, the spec -strip and artwork. +strip and artwork. `ARTIST` resolves `tracks.artist_id`; `ALBUMARTIST` (or +`ARTIST` when absent) resolves the album's `artist_id`; and `ALBUM` resolves +`tracks.album_id`. This keeps the normalised tables and the track metadata in +step after a tag edit. The queue is not a table: it is `tracks` where `last_scanned_at IS NULL`. Each batch is written before the next starts, so a scan that is cancelled, crashes diff --git a/docs/stats.md b/docs/stats.md index 699ba14..72b3342 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -186,10 +186,13 @@ week/month/year × track/artist/album/playlist incrementally, from `applyRollups` inside `recordListen`. One listen writes up to twelve cells — three periods times four entities, -minus whatever is null. `rollupDeltas` builds that as a product rather than by -hand, so adding a period or an entity type cannot be done to one and forgotten -in the others. Null artist and album ids are **skipped**, never defaulted to -zero: an untagged file must not accumulate against a phantom entity. +minus a missing playlist. `rollupDeltas` builds that as a product rather than +by hand, so adding a period or an entity type cannot be done to one and +forgotten in the others. A null artist or album uses the reserved rollup id +`0`: it cannot collide with SQLite's positive row ids, is left-joined at read +time, and is rendered as the active locale's “Unknown Artist” or “Unknown +Album”. The content tables still retain null — no translated phantom row is +stored in user data. The artist and album come from the `tracks` row, not from the caller. The player knows what it is playing, not how the library has it classified, and a diff --git a/src/components/ui/SegmentedControl.tsx b/src/components/ui/SegmentedControl.tsx index 598a38c..69dfd6f 100644 --- a/src/components/ui/SegmentedControl.tsx +++ b/src/components/ui/SegmentedControl.tsx @@ -59,6 +59,7 @@ export function SegmentedControl({ onChange(option.value)} + android_ripple={{ color: selected ? colors.onSignal : colors.etch }} accessibilityRole="radio" accessibilityState={{ selected }} accessibilityLabel={option.label} diff --git a/src/components/ui/TabErrorBoundary.tsx b/src/components/ui/TabErrorBoundary.tsx new file mode 100644 index 0000000..22a0116 --- /dev/null +++ b/src/components/ui/TabErrorBoundary.tsx @@ -0,0 +1,48 @@ +import { Component, type ErrorInfo, type ReactNode } from 'react'; +import { View } from 'react-native'; + +import { i18n } from '@/i18n'; + +import { ErrorState } from './ErrorState'; + +export interface TabErrorBoundaryProps { + children: ReactNode; +} + +interface TabErrorBoundaryState { + hasError: boolean; +} + +/** Keeps a render failure inside one tab instead of taking down the whole app. */ +// React render error boundaries require a class component. +export class TabErrorBoundary extends Component { + override state: TabErrorBoundaryState = { hasError: false }; + + static getDerivedStateFromError(): TabErrorBoundaryState { + return { hasError: true }; + } + + override componentDidCatch(error: Error, info: ErrorInfo): void { + if (__DEV__) console.error('Tab render failed:', error, info.componentStack); + } + + private retry = (): void => { + this.setState({ hasError: false }); + }; + + override render(): ReactNode { + if (this.state.hasError) { + return ( + + + + ); + } + + return this.props.children; + } +} diff --git a/src/db/queries/scanning.ts b/src/db/queries/scanning.ts index 365f370..9f87eca 100644 Binary files a/src/db/queries/scanning.ts and b/src/db/queries/scanning.ts differ diff --git a/src/db/queries/stats.ts b/src/db/queries/stats.ts index 8310c55..acec217 100644 --- a/src/db/queries/stats.ts +++ b/src/db/queries/stats.ts @@ -17,7 +17,8 @@ import { albums, artists, playlists, statsRollups, tracks } from '../schema'; export interface TopEntry { id: number; - title: string; + /** Null only for the reserved unknown artist or album rollup row. */ + title: string | null; subtitle: string | null; playCount: number; msPlayed: number; @@ -100,7 +101,7 @@ export function useTopTracks(periodType: PeriodType, periodKey: string, limit = export function useTopArtists(periodType: PeriodType, periodKey: string, limit = 10) { const query = db .select({ - id: artists.id, + id: statsRollups.entityId, title: artists.name, subtitle: sql`null`, playCount: statsRollups.playCount, @@ -108,7 +109,7 @@ export function useTopArtists(periodType: PeriodType, periodKey: string, limit = artworkPath: artistCover, }) .from(statsRollups) - .innerJoin(artists, eq(artists.id, statsRollups.entityId)) + .leftJoin(artists, eq(artists.id, statsRollups.entityId)) .where(rankedRollups(periodType, periodKey, 'artist')) .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) .limit(limit); @@ -121,7 +122,7 @@ export function useTopArtists(periodType: PeriodType, periodKey: string, limit = export function useTopAlbums(periodType: PeriodType, periodKey: string, limit = 10) { const query = db .select({ - id: albums.id, + id: statsRollups.entityId, title: albums.name, subtitle: artists.name, playCount: statsRollups.playCount, @@ -129,7 +130,7 @@ export function useTopAlbums(periodType: PeriodType, periodKey: string, limit = artworkPath: albumCover, }) .from(statsRollups) - .innerJoin(albums, eq(albums.id, statsRollups.entityId)) + .leftJoin(albums, eq(albums.id, statsRollups.entityId)) .leftJoin(artists, eq(artists.id, albums.artistId)) .where(rankedRollups(periodType, periodKey, 'album')) .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) diff --git a/src/db/queries/tracks.ts b/src/db/queries/tracks.ts index 745a6bf..3b23afc 100644 --- a/src/db/queries/tracks.ts +++ b/src/db/queries/tracks.ts @@ -1,4 +1,4 @@ -import { and, asc, count, eq, like, or, sql } from 'drizzle-orm'; +import { and, asc, count, eq, isNull, like, or, sql } from 'drizzle-orm'; import { useLiveQuery } from 'drizzle-orm/expo-sqlite'; import { useEffect } from 'react'; @@ -266,9 +266,12 @@ export async function markMissing(trackIds: number[]): Promise { /** An artist or album as a card: cover, name, and how much of it there is. */ export interface CollectionCard { id: number; - name: string; + /** Null only for the reserved unknown artist or album card. */ + name: string | null; /** For an album, its artist. Null for an artist card. */ subtitle: string | null; + isUnknown: boolean; + isUnknownSubtitle: boolean; trackCount: number; artworkPath: string | null; } @@ -286,18 +289,22 @@ export interface CollectionCard { * artwork changes between renders looks broken. */ export function useArtistCards(): CollectionCard[] { + const collectionId = sql`coalesce(${tracks.artistId}, 0)`; const query = db .select({ - id: artists.id, + id: collectionId, name: artists.name, subtitle: sql`null`, + isUnknown: sql`${tracks.artistId} IS NULL`.mapWith(Boolean), + isUnknownSubtitle: sql`false`.mapWith(Boolean), trackCount: count(tracks.id), artworkPath: sql`min(${tracks.artworkPath})`, }) - .from(artists) - .innerJoin(tracks, and(eq(tracks.artistId, artists.id), eq(tracks.isMissing, 0))) - .groupBy(artists.id) - .orderBy(asc(sql`${artists.sortName} COLLATE NOCASE`)); + .from(tracks) + .leftJoin(artists, eq(tracks.artistId, artists.id)) + .where(eq(tracks.isMissing, 0)) + .groupBy(collectionId) + .orderBy(asc(sql`coalesce(${artists.sortName}, '') COLLATE NOCASE`)); const { data } = useLiveQuery(query); return useThrottledData(data); @@ -305,19 +312,25 @@ export function useArtistCards(): CollectionCard[] { /** Every album that has at least one present track. */ export function useAlbumCards(): CollectionCard[] { + const collectionId = sql`coalesce(${tracks.albumId}, 0)`; const query = db .select({ - id: albums.id, + id: collectionId, name: albums.name, subtitle: artists.name, + isUnknown: sql`${tracks.albumId} IS NULL`.mapWith(Boolean), + isUnknownSubtitle: sql`${tracks.albumId} IS NOT NULL AND ${albums.artistId} IS NULL`.mapWith( + Boolean, + ), trackCount: count(tracks.id), artworkPath: sql`min(${tracks.artworkPath})`, }) - .from(albums) - .innerJoin(tracks, and(eq(tracks.albumId, albums.id), eq(tracks.isMissing, 0))) + .from(tracks) + .leftJoin(albums, eq(tracks.albumId, albums.id)) .leftJoin(artists, eq(artists.id, albums.artistId)) - .groupBy(albums.id) - .orderBy(asc(sql`${albums.name} COLLATE NOCASE`)); + .where(eq(tracks.isMissing, 0)) + .groupBy(collectionId) + .orderBy(asc(sql`coalesce(${albums.name}, '') COLLATE NOCASE`)); const { data } = useLiveQuery(query); return useThrottledData(data); @@ -331,6 +344,14 @@ export function useAlbumCards(): CollectionCard[] { * partially-tagged record still opens with the tracks that know where they go. */ export function useCollectionTracks(kind: 'artist' | 'album', id: number): TrackListItem[] { + const collectionPredicate = + kind === 'artist' + ? id === 0 + ? isNull(tracks.artistId) + : eq(tracks.artistId, id) + : id === 0 + ? isNull(tracks.albumId) + : eq(tracks.albumId, id); const query = db .select(listSelection) .from(tracks) @@ -340,7 +361,7 @@ export function useCollectionTracks(kind: 'artist' | 'album', id: number): Track .where( and( eq(tracks.isMissing, 0), - kind === 'artist' ? eq(tracks.artistId, id) : eq(tracks.albumId, id), + collectionPredicate, ), ) .orderBy( diff --git a/src/db/seed.ts b/src/db/seed.ts index 3621cd0..4f79ea0 100644 --- a/src/db/seed.ts +++ b/src/db/seed.ts @@ -1,7 +1,7 @@ import { count } from 'drizzle-orm'; import { db } from './client'; -import { albums, artists, tracks } from './schema'; +import { albums, artists, playEvents, playlistTracks, statsRollups, trackStats, tracks } from './schema'; /* * Fake library for UI work, so Phase 4 can build lists before Phase 2 can @@ -136,11 +136,18 @@ export async function seedDatabase(): Promise { return inserted; } -/** Remove everything the seed inserted. Development only. */ +/** Clear generated tracks and every record that would otherwise refer to them. Development only. */ export async function clearDatabase(): Promise { - await db.delete(tracks); - await db.delete(albums); - await db.delete(artists); + await db.transaction(async (tx) => { + // Rollups are not foreign-key children, so SQLite cannot cascade them. + await tx.delete(statsRollups); + await tx.delete(playEvents); + await tx.delete(trackStats); + await tx.delete(playlistTracks); + await tx.delete(tracks); + await tx.delete(albums); + await tx.delete(artists); + }); } /* diff --git a/src/features/library/CollectionDetailScreen.tsx b/src/features/library/CollectionDetailScreen.tsx index 6768f7d..30585c3 100644 --- a/src/features/library/CollectionDetailScreen.tsx +++ b/src/features/library/CollectionDetailScreen.tsx @@ -83,6 +83,8 @@ export function CollectionDetailScreen({ kind, id }: CollectionDetailScreenProps kind={kind} name={card.name} subtitle={card.subtitle} + isUnknown={card.isUnknown} + isUnknownSubtitle={card.isUnknownSubtitle} trackCount={tracks.length} artworkPath={card.artworkPath} /> diff --git a/src/features/library/LibraryScreen.tsx b/src/features/library/LibraryScreen.tsx index 781bb45..7b5b756 100644 --- a/src/features/library/LibraryScreen.tsx +++ b/src/features/library/LibraryScreen.tsx @@ -58,6 +58,11 @@ export function LibraryScreen() { const artists = useArtistCards(); const albums = useAlbumCards(); const { openArtist, openAlbum } = useCollectionRouting(); + const collectionCards = view === 'artists' ? artists : albums; + const displayedTrackCount = + view === 'tracks' + ? tracks.length + : collectionCards.reduce((total, card) => total + card.trackCount, 0); /** True while the scan confirmation is on screen. */ const [confirmingScan, setConfirmingScan] = useState(false); @@ -87,7 +92,7 @@ export function LibraryScreen() { return ( ) : ( new Set(selectedIdList), [selectedIdList]); const { addToQueue, playNext, toggleFavorite } = useTrackActions(); + // Queue conversion belongs to a data change, not to a row press. + const playableTracks = useMemo(() => { + perf.mark('library.playableQueue'); + const playable = tracks.map(toPlayable); + perf.measure('library.playableQueue', playable.length); + return playable; + }, [tracks]); + const trackIndexById = useMemo( + () => new Map(tracks.map((track, index) => [track.id, index])), + [tracks], + ); /** Which track's action sheet is open. */ const [actionTarget, setActionTarget] = useState(null); @@ -92,11 +104,14 @@ export function LibraryTracks({ toggleSelected(id); return; } - const index = tracks.findIndex((track) => track.id === id); - if (index === -1) return; - playFrom(tracks.map(toPlayable), index); + const index = trackIndexById.get(id); + if (index === undefined) return; + perf.mark('library.play.handler'); + perf.mark('library.play.toMiniPlayer'); + playFrom(playableTracks, index); + perf.measure('library.play.handler', playableTracks.length); }, - [tracks, playFrom, isSelecting, toggleSelected], + [trackIndexById, playFrom, playableTracks, isSelecting, toggleSelected], ); const onLongPress = useCallback( diff --git a/src/features/library/components/CollectionCard.tsx b/src/features/library/components/CollectionCard.tsx index 67953ca..c60c29a 100644 --- a/src/features/library/components/CollectionCard.tsx +++ b/src/features/library/components/CollectionCard.tsx @@ -8,6 +8,7 @@ import type { CollectionCard as Card } from '@/db/queries/tracks'; import { useThemeColors } from '@/theme/useTheme'; export interface CollectionCardProps { + kind: 'artist' | 'album'; card: Card; /** Drawn when there is no cover. A disc for albums, a person for artists. */ icon: LucideIcon; @@ -27,6 +28,7 @@ export interface CollectionCardProps { * the covers rounder than the panel they sit on. */ const CollectionCardComponent = function CollectionCard({ + kind, card, icon: Icon, onPress, @@ -35,13 +37,21 @@ const CollectionCardComponent = function CollectionCard({ const colors = useThemeColors(); const handlePress = useCallback(() => onPress(card.id), [onPress, card.id]); - const subtitle = card.subtitle ?? t('library.trackCount', { count: card.trackCount }); + const name = card.isUnknown + ? t(kind === 'artist' ? 'common.unknownArtist' : 'common.unknownAlbum') + : card.name ?? t(kind === 'artist' ? 'common.unknownArtist' : 'common.unknownAlbum'); + const subtitle = card.isUnknown + ? t('library.trackCount', { count: card.trackCount }) + : card.isUnknownSubtitle + ? t('common.unknownArtist') + : card.subtitle ?? t('library.trackCount', { count: card.trackCount }); return ( @@ -62,7 +72,7 @@ const CollectionCardComponent = function CollectionCard({ - {card.name} + {name} {subtitle} @@ -74,11 +84,14 @@ const CollectionCardComponent = function CollectionCard({ function isSameCard(previous: CollectionCardProps, next: CollectionCardProps): boolean { return ( + previous.kind === next.kind && previous.onPress === next.onPress && previous.icon === next.icon && previous.card.id === next.card.id && previous.card.name === next.card.name && previous.card.subtitle === next.card.subtitle && + previous.card.isUnknown === next.card.isUnknown && + previous.card.isUnknownSubtitle === next.card.isUnknownSubtitle && previous.card.trackCount === next.card.trackCount && previous.card.artworkPath === next.card.artworkPath ); diff --git a/src/features/library/components/CollectionGrid.tsx b/src/features/library/components/CollectionGrid.tsx index 74bb40d..ec7438d 100644 --- a/src/features/library/components/CollectionGrid.tsx +++ b/src/features/library/components/CollectionGrid.tsx @@ -12,6 +12,7 @@ import { CollectionCard } from './CollectionCard'; const COLUMNS = 2; export interface CollectionGridProps { + kind: 'artist' | 'album'; cards: readonly Card[]; icon: LucideIcon; onPress: (id: number) => void; @@ -31,16 +32,16 @@ export interface CollectionGridProps { * be wrong on the first rotation. Cards are uniform, so FlashList measures one * and reuses it. */ -export function CollectionGrid({ cards, icon, onPress, empty }: CollectionGridProps) { +export function CollectionGrid({ kind, cards, icon, onPress, empty }: CollectionGridProps) { const renderItem = useCallback>( ({ item }) => ( // Gutter as padding on the cell rather than a gap on the list: FlashList // sizes cells itself, and a gap would be applied outside that measurement. - + ), - [icon, onPress], + [kind, icon, onPress], ); return ( diff --git a/src/features/library/components/CollectionHeader.tsx b/src/features/library/components/CollectionHeader.tsx index 85ef258..b8ceb4d 100644 --- a/src/features/library/components/CollectionHeader.tsx +++ b/src/features/library/components/CollectionHeader.tsx @@ -7,9 +7,11 @@ import { useThemeColors } from '@/theme/useTheme'; export interface CollectionHeaderProps { kind: 'artist' | 'album'; - name: string; + name: string | null; /** The album's artist. Null for an artist. */ subtitle: string | null; + isUnknown: boolean; + isUnknownSubtitle: boolean; trackCount: number; artworkPath: string | null; } @@ -19,19 +21,25 @@ export function CollectionHeader({ kind, name, subtitle, + isUnknown, + isUnknownSubtitle, trackCount, artworkPath, }: CollectionHeaderProps) { const { t } = useTranslation(); const colors = useThemeColors(); const Icon = kind === 'artist' ? User : Disc3; + const displayName = isUnknown + ? t(kind === 'artist' ? 'common.unknownArtist' : 'common.unknownAlbum') + : name ?? t(kind === 'artist' ? 'common.unknownArtist' : 'common.unknownAlbum'); + const displaySubtitle = isUnknown ? null : isUnknownSubtitle ? t('common.unknownArtist') : subtitle; return ( {artworkPath ? ( - {name} + {displayName} - {subtitle ? ( + {displaySubtitle ? ( - {subtitle} + {displaySubtitle} ) : null} diff --git a/src/features/library/components/LibraryHeader.tsx b/src/features/library/components/LibraryHeader.tsx index 3b2665f..bd55145 100644 --- a/src/features/library/components/LibraryHeader.tsx +++ b/src/features/library/components/LibraryHeader.tsx @@ -51,6 +51,7 @@ export function LibraryHeader({ onSelect(id as TrackAction)} diff --git a/src/features/library/components/TrackInfoSheet.tsx b/src/features/library/components/TrackInfoSheet.tsx index f274fee..5039e4e 100644 --- a/src/features/library/components/TrackInfoSheet.tsx +++ b/src/features/library/components/TrackInfoSheet.tsx @@ -29,8 +29,8 @@ export function TrackInfoSheet({ track, onClose }: TrackInfoSheetProps) { const rows: [string, string][] = [ [t('track.field.title'), track?.title ?? '—'], - [t('track.field.artist'), track?.artistName ?? '—'], - [t('track.field.album'), track?.albumName ?? '—'], + [t('track.field.artist'), track ? (track.artistName ?? t('common.unknownArtist')) : '—'], + [t('track.field.album'), track ? (track.albumName ?? t('common.unknownAlbum')) : '—'], [t('track.field.duration'), track ? formatDuration(track.durationMs, i18n.language) : '—'], [t('track.field.codec'), spec?.codec?.toUpperCase() ?? '—'], [t('track.field.container'), spec?.container ?? '—'], diff --git a/src/features/library/components/TrackRow.tsx b/src/features/library/components/TrackRow.tsx index bb0e40c..174508c 100644 --- a/src/features/library/components/TrackRow.tsx +++ b/src/features/library/components/TrackRow.tsx @@ -1,6 +1,7 @@ import { Image } from 'expo-image'; import { Check, Music } from 'lucide-react-native'; import { memo, useCallback } from 'react'; +import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; import type { TrackListItem } from '@/db/queries/tracks'; @@ -38,6 +39,7 @@ const TrackRowComponent = function TrackRow({ isSelected = false, isCurrent = false, }: TrackRowProps) { + const { t } = useTranslation(); const colors = useThemeColors(); const handlePress = useCallback(() => onPress(track.id), [onPress, track.id]); const handleLongPress = useCallback(() => onLongPress(track.id), [onLongPress, track.id]); @@ -46,12 +48,16 @@ const TrackRowComponent = function TrackRow({ // Kotlin side writes and what it hands back. expo-image needs the scheme. const artworkUri = track.artworkPath ? `file://${track.artworkPath}` : null; - const subtitle = [track.artistName, track.albumName].filter(Boolean).join(' — '); + const subtitle = [ + track.artistName ?? t('common.unknownArtist'), + track.albumName ?? t('common.unknownAlbum'), + ].join(' — '); return ( - {track.artistName ?? t('player.unknownArtist')} + {track.artistName ?? t('common.unknownArtist')} diff --git a/src/features/player/components/MiniPlayer.tsx b/src/features/player/components/MiniPlayer.tsx index b759bc0..e886d57 100644 --- a/src/features/player/components/MiniPlayer.tsx +++ b/src/features/player/components/MiniPlayer.tsx @@ -1,7 +1,7 @@ import { Image } from 'expo-image'; import { useRouter } from 'expo-router'; import { Music, Pause, Play, SkipBack, SkipForward } from 'lucide-react-native'; -import { useCallback } from 'react'; +import { useCallback, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; @@ -13,6 +13,7 @@ import Animated, { } from 'react-native-reanimated'; import { tapFeedback } from '@/services/haptics'; +import * as perf from '@/services/perf'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { useReducedMotion } from '@/theme/useReducedMotion'; import { useThemeColors } from '@/theme/useTheme'; @@ -75,6 +76,10 @@ export function MiniPlayer() { const track = useCurrentTrack(); const { toggle, next, previous } = usePlaybackControls(); + useEffect(() => { + if (track !== null) perf.measure('library.play.toMiniPlayer', track.id); + }, [track]); + /* * `navigate`, not `push`. * @@ -104,9 +109,10 @@ export function MiniPlayer() { * routinely lost the race to a child's press responder, so the swipe worked * sometimes and looked broken the rest of the time. * - * This one activates on vertical movement only (`activeOffsetY`) and treats - * horizontal travel as a secondary read once it already owns the gesture, so - * it never competes with a tap and never needs to. + * A small minimum distance lets the pan claim both axes while leaving taps + * to the child Pressables. `activeOffsetY` looked safer but meant a purely + * horizontal drag could never activate, so mini-player track swipes did not + * exist in practice. * * Built inline rather than memoized, like `Scrubber` and unlike * `SwipeableRow`: there is exactly one mini player, so a rebuild per render @@ -115,7 +121,7 @@ export function MiniPlayer() { * values and simply does not model Reanimated. */ const pan = Gesture.Pan() - .activeOffsetY([-ACTIVATION_SLOP, ACTIVATION_SLOP]) + .minDistance(ACTIVATION_SLOP) .onBegin(() => { axis.value = 0; }) @@ -180,6 +186,7 @@ export function MiniPlayer() { {track.title} - {track.artistName ? ( - - {track.artistName} - - ) : null} + + {track.artistName ?? t('common.unknownArtist')} + @@ -218,6 +223,7 @@ export function MiniPlayer() { */} onRemove(position), [onRemove, position]); const artworkUri = track.artworkPath ? `file://${track.artworkPath}` : null; - const subtitle = [track.artistName, track.albumName].filter(Boolean).join(' — '); + const subtitle = [ + track.artistName ?? t('common.unknownArtist'), + track.albumName ?? t('common.unknownAlbum'), + ].join(' — '); return ( onToggle(track.id), [onToggle, track.id]); + const subtitle = [ + track.artistName ?? t('common.unknownArtist'), + track.albumName ?? t('common.unknownAlbum'), + ].join(' — '); return ( onToggle(track.id)} + onPress={handlePress} accessibilityRole="checkbox" accessibilityLabel={track.title} accessibilityHint={subtitle || undefined} @@ -168,8 +173,21 @@ function PickRow({ track, isPicked, onToggle }: PickRowProps) { ); +}; + +function isSamePickRow(previous: PickRowProps, next: PickRowProps): boolean { + return ( + previous.isPicked === next.isPicked && + previous.onToggle === next.onToggle && + previous.track.id === next.track.id && + previous.track.title === next.track.title && + previous.track.artistName === next.track.artistName && + previous.track.albumName === next.track.albumName + ); } +const PickRow = memo(PickRowComponent, isSamePickRow); + function keyExtractor(track: TrackListItem): string { return String(track.id); } diff --git a/src/features/playlists/components/PlaylistEntryRow.tsx b/src/features/playlists/components/PlaylistEntryRow.tsx index ca89126..313be32 100644 --- a/src/features/playlists/components/PlaylistEntryRow.tsx +++ b/src/features/playlists/components/PlaylistEntryRow.tsx @@ -34,7 +34,10 @@ export const PlaylistEntryRow = memo(function PlaylistEntryRow({ const handleRemove = useCallback(() => onRemove(entry.position), [onRemove, entry.position]); const artworkUri = entry.artworkPath ? `file://${entry.artworkPath}` : null; - const subtitle = [entry.artistName, entry.albumName].filter(Boolean).join(' — '); + const subtitle = [ + entry.artistName ?? t('common.unknownArtist'), + entry.albumName ?? t('common.unknownAlbum'), + ].join(' — '); return ( diff --git a/src/features/stats/StatsScreen.tsx b/src/features/stats/StatsScreen.tsx index 460392e..42e34d6 100644 --- a/src/features/stats/StatsScreen.tsx +++ b/src/features/stats/StatsScreen.tsx @@ -75,13 +75,24 @@ export function StatsScreen() { totals={totals} topTrack={topTracks[0]} topArtist={topArtists[0]} + unknownArtist={t('common.unknownArtist')} /> - - + + ) : ( diff --git a/src/features/stats/components/TopList.tsx b/src/features/stats/components/TopList.tsx index db076e6..e916078 100644 --- a/src/features/stats/components/TopList.tsx +++ b/src/features/stats/components/TopList.tsx @@ -13,6 +13,8 @@ export interface TopListProps { entries: readonly TopEntry[]; /** Drawn when an entry has no cover. Says what kind of thing this list holds. */ icon: LucideIcon; + /** Used only by the reserved unknown artist or album row. */ + unknownTitle?: string; } /** @@ -29,7 +31,7 @@ export interface TopListProps { * measure and loses badly on the other. Showing only the count was hiding half * of what `stats_rollups` already knew. */ -export function TopList({ title, entries, icon: Icon }: TopListProps) { +export function TopList({ title, entries, icon: Icon, unknownTitle }: TopListProps) { const { t, i18n } = useTranslation(); const colors = useThemeColors(); @@ -64,7 +66,7 @@ export function TopList({ title, entries, icon: Icon }: TopListProps) { - {entry.title} + {entry.title ?? unknownTitle} {/* diff --git a/src/features/stats/components/Wrapped.tsx b/src/features/stats/components/Wrapped.tsx index a32e40e..be9e54c 100644 --- a/src/features/stats/components/Wrapped.tsx +++ b/src/features/stats/components/Wrapped.tsx @@ -10,6 +10,7 @@ export interface WrappedProps { totals: PeriodTotals; topTrack: TopEntry | undefined; topArtist: TopEntry | undefined; + unknownArtist: string; } /** @@ -30,7 +31,7 @@ export interface WrappedProps { * intent in an app whose whole promise is that nothing leaves the device. A * screenshot is already the share mechanism, and it needs no permission. */ -export function Wrapped({ period, totals, topTrack, topArtist }: WrappedProps) { +export function Wrapped({ period, totals, topTrack, topArtist, unknownArtist }: WrappedProps) { const { t, i18n } = useTranslation(); const time = formatListeningTime(totals.msPlayed, i18n.language); @@ -61,10 +62,10 @@ export function Wrapped({ period, totals, topTrack, topArtist }: WrappedProps) { {topTrack || topArtist ? ( {topTrack ? ( - + ) : null} {topArtist ? ( - + ) : null} ) : null} diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 3d4380a..170d9be 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -183,7 +183,11 @@ "cancel": "Cancel", "save": "Save", "back": "Back", - "close": "Close" + "close": "Close", + "unknownArtist": "Unknown artist", + "unknownAlbum": "Unknown album", + "unexpectedError": "Something went wrong. Try again.", + "tryAgain": "Try again" }, "queue": { "title": "Queue", diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index 56897da..b7363eb 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -183,7 +183,11 @@ "cancel": "Vazgeç", "save": "Kaydet", "back": "Geri", - "close": "Kapat" + "close": "Kapat", + "unknownArtist": "Bilinmeyen sanatçı", + "unknownAlbum": "Bilinmeyen albüm", + "unexpectedError": "Bir şeyler ters gitti. Tekrar dene.", + "tryAgain": "Tekrar dene" }, "queue": { "title": "Sıra", diff --git a/src/services/audio/AudioEngine.ts b/src/services/audio/AudioEngine.ts index d11a233..adc886b 100644 --- a/src/services/audio/AudioEngine.ts +++ b/src/services/audio/AudioEngine.ts @@ -8,6 +8,7 @@ import { } from 'expo-audio'; import { shuffleTracks, type ShuffleAlgorithm } from '@/services/shuffle'; +import { ListenCycle, type BankedListen } from '@/services/stats/listenCycle'; import { isRewindToRestart } from '@/services/stats/repeatListen'; import { isPlayable, nextIndex, playNextIndex, previousIndex, shiftForInsert } from './queue'; @@ -96,9 +97,7 @@ class Engine { * play/skip rule would count it as a full listen. */ private reportListen: ListenReporter | null = null; - private startedAt: Date | null = null; - private playedMs = 0; - private lastTickAt: number | null = null; + private listenCycle = new ListenCycle(); /** * Position at the previous status tick, for spotting a rewind. @@ -321,25 +320,23 @@ class Engine { /** Bank the time played so far and hand the listen over. */ private flushListen(completed: boolean): void { - this.accumulate(); + this.reportClosedListen(this.listenCycle.close(), completed); + } + /** Send an already-banked cycle to statistics while its track is still current. */ + private reportClosedListen(listen: BankedListen | null, completed: boolean): void { const track = this.state.track; - const startedAt = this.startedAt; - if (track !== null && startedAt !== null && this.playedMs > 0) { + if (track !== null && listen !== null) { this.reportListen?.({ track, - msPlayed: Math.round(this.playedMs), - startedAt, + msPlayed: listen.msPlayed, + startedAt: listen.startedAt, completed, source: this.source, shuffleAlgorithm: this.shuffleAlgorithm, }); } - - this.startedAt = null; - this.playedMs = 0; - this.lastTickAt = null; } /** @@ -351,15 +348,7 @@ class Engine { * its two halves in the right days. */ private beginNextCycle(): void { - this.flushListen(true); - this.startedAt = new Date(); - } - - /** Fold the time since the last tick into the running total. */ - private accumulate(): void { - if (this.lastTickAt === null) return; - this.playedMs += Date.now() - this.lastTickAt; - this.lastTickAt = null; + this.reportClosedListen(this.listenCycle.restart(), true); } private async loadIndex(index: number, autoPlay: boolean): Promise { @@ -368,7 +357,7 @@ class Engine { // The outgoing track's listen closes before the incoming one starts. this.flushListen(false); - this.startedAt = new Date(); + this.listenCycle.open(); this.index = index; this.lastPositionMs = 0; @@ -438,12 +427,7 @@ class Engine { const positionMs = Math.round(status.currentTime * 1000); // Clock the interval that just elapsed before anything else changes. - if (status.playing) { - this.accumulate(); - this.lastTickAt = Date.now(); - } else { - this.accumulate(); - } + this.listenCycle.tick(status.playing); // A track that reached its end advances the queue. `didJustFinish` fires // once, unlike `currentTime >= duration`, which fires on every tick after. @@ -467,7 +451,7 @@ class Engine { previousPositionMs: this.lastPositionMs, positionMs, durationMs: this.state.durationMs, - msPlayedInCycle: this.playedMs, + msPlayedInCycle: this.listenCycle.msPlayedInCycle, }) ) { this.beginNextCycle(); diff --git a/src/services/perf/index.ts b/src/services/perf/index.ts index be15699..6a1f8d7 100644 --- a/src/services/perf/index.ts +++ b/src/services/perf/index.ts @@ -16,6 +16,11 @@ const TAG = 'MUFIFY_PERF'; const counters = new Map(); const marks = new Map(); +/** Hermes exposes a monotonic sub-millisecond clock; Date is the fallback. */ +function now(): number { + return globalThis.performance?.now?.() ?? Date.now(); +} + /** * Count an occurrence and log the running total. * @@ -33,7 +38,7 @@ export function count(label: string): void { /** Start a stopwatch. Overwrites any unfinished one under the same label. */ export function mark(label: string): void { if (!__DEV__) return; - marks.set(label, Date.now()); + marks.set(label, now()); } /** @@ -53,8 +58,10 @@ export function measure(label: string, detail?: string | number): number { } marks.delete(label); - const elapsed = Date.now() - started; - console.log(`${TAG} measure ${label} ${elapsed}ms${detail === undefined ? '' : ` ${detail}`}`); + const elapsed = now() - started; + console.log( + `${TAG} measure ${label} ${elapsed.toFixed(1)}ms${detail === undefined ? '' : ` ${detail}`}`, + ); return elapsed; } diff --git a/src/services/scanner/trackMapping.test.ts b/src/services/scanner/trackMapping.test.ts index 9330f44..2bd5fbc 100644 --- a/src/services/scanner/trackMapping.test.ts +++ b/src/services/scanner/trackMapping.test.ts @@ -189,6 +189,9 @@ describe('fromTags', () => { it('maps the spec strip fields', () => { const enriched = fromTags(tagRow()); expect(enriched).toMatchObject({ + artistName: 'Barış Manço', + albumName: 'Sakla Samanı', + albumArtist: 'Barış Manço', container: 'FLAC', // Null rather than 'flac': container and codec read the same MIME // subtype, so repeating it produced the strip "FLAC · flac". @@ -215,6 +218,15 @@ describe('fromTags', () => { it('omits the title when the file has none, so MediaStore keeps its own', () => { expect(fromTags(tagRow({ title: null }))).not.toHaveProperty('title'); }); + + it('keeps missing collection tags null so they become one fallback category', () => { + const enriched = fromTags(tagRow({ artist: null, album: null, albumArtist: null })); + expect(enriched).toMatchObject({ + artistName: null, + albumName: null, + albumArtist: null, + }); + }); }); describe('containerOf', () => { diff --git a/src/services/scanner/trackMapping.ts b/src/services/scanner/trackMapping.ts index 256909a..b80ce20 100644 --- a/src/services/scanner/trackMapping.ts +++ b/src/services/scanner/trackMapping.ts @@ -84,6 +84,9 @@ export function fromMediaStore(row: MediaStoreTrack): ScannedTrack { export interface EnrichedFields { title?: string; + artistName: string | null; + albumName: string | null; + albumArtist: string | null; genre: string | null; trackNo: number | null; discNo: number | null; @@ -111,6 +114,9 @@ export function fromTags(tags: TrackTags): EnrichedFields | null { return { ...(tags.title?.trim() ? { title: tags.title.trim() } : {}), + artistName: blankToNull(tags.artist), + albumName: blankToNull(tags.album), + albumArtist: blankToNull(tags.albumArtist), genre: blankToNull(tags.genre), trackNo, discNo: tags.discNumber ?? discNo, diff --git a/src/services/stats/listenCycle.test.ts b/src/services/stats/listenCycle.test.ts new file mode 100644 index 0000000..8e44fdc --- /dev/null +++ b/src/services/stats/listenCycle.test.ts @@ -0,0 +1,101 @@ +import { ListenCycle } from './listenCycle'; + +/** A fixed clock, so nothing here depends on how fast the test runs. */ +const T0 = 1_700_000_000_000; + +describe('ListenCycle', () => { + it('is closed before anything opens it', () => { + const cycle = new ListenCycle(); + expect(cycle.isOpen).toBe(false); + expect(cycle.close(T0)).toBeNull(); + }); + + it('accumulates only the intervals it was playing', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + + cycle.tick(true, T0); // clock starts + cycle.tick(true, T0 + 10_000); // +10s played + cycle.tick(false, T0 + 15_000); // +5s played, then paused + cycle.tick(false, T0 + 60_000); // paused throughout, adds nothing + cycle.tick(true, T0 + 60_000); // clock restarts + cycle.tick(true, T0 + 62_000); // +2s played + + expect(cycle.close(T0 + 62_000)).toEqual({ + startedAt: new Date(T0), + msPlayed: 17_000, + }); + }); + + it('reports nothing for a cycle that never played', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + expect(cycle.close(T0 + 5_000)).toBeNull(); + }); + + it('closes the cycle so a second close reports nothing', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + cycle.tick(true, T0); + cycle.tick(true, T0 + 30_000); + + expect(cycle.close(T0 + 30_000)?.msPlayed).toBe(30_000); + expect(cycle.isOpen).toBe(false); + expect(cycle.close(T0 + 30_000)).toBeNull(); + }); + + describe('restart — the repeat-one regression', () => { + it('leaves a cycle open, so the next loop can still be banked', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + cycle.tick(true, T0); + cycle.tick(true, T0 + 200_000); + + cycle.restart(T0 + 200_000); + + // The bug was here: close() alone left no open cycle, and every + // subsequent loop was silently discarded. + expect(cycle.isOpen).toBe(true); + }); + + it('banks each pass of a track looped five times', () => { + const cycle = new ListenCycle(); + const banked: number[] = []; + const trackMs = 200_000; + + cycle.open(new Date(T0)); + for (let loop = 0; loop < 5; loop += 1) { + const from = T0 + loop * trackMs; + cycle.tick(true, from); + cycle.tick(true, from + trackMs); + const listen = cycle.restart(from + trackMs); + if (listen) banked.push(listen.msPlayed); + } + + expect(banked).toEqual([trackMs, trackMs, trackMs, trackMs, trackMs]); + }); + + it('dates each pass from when that pass began, not when the first did', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + cycle.tick(true, T0); + cycle.tick(true, T0 + 200_000); + + const first = cycle.restart(T0 + 200_000); + cycle.tick(true, T0 + 200_000); + cycle.tick(true, T0 + 400_000); + const second = cycle.close(T0 + 400_000); + + expect(first?.startedAt).toEqual(new Date(T0)); + // A loop crossing midnight has to put its halves in different days. + expect(second?.startedAt).toEqual(new Date(T0 + 200_000)); + }); + + it('does not bank a restart that played nothing', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + expect(cycle.restart(T0 + 1_000)).toBeNull(); + expect(cycle.isOpen).toBe(true); + }); + }); +}); diff --git a/src/services/stats/listenCycle.ts b/src/services/stats/listenCycle.ts new file mode 100644 index 0000000..88a051e --- /dev/null +++ b/src/services/stats/listenCycle.ts @@ -0,0 +1,104 @@ +/** + * One listen, from the moment it opens to the moment it is banked. + * + * Pulled out of `AudioEngine` because a state bug lived here undetected: the + * engine closed a listen when a track finished but only reopened one inside + * `loadIndex`, and repeat-one never calls `loadIndex` — it seeks to zero and + * plays the same file again. So the first loop was recorded and every loop + * after it was dropped on the floor, because banking requires an open cycle + * and nothing had opened one. + * + * The engine cannot be unit tested without a real audio session. This can: + * it holds no player, imports nothing from Android, and takes the clock as an + * argument. The rule that broke is now a rule something asserts. + * + * Time is accumulated tick by tick rather than read off the final position, + * because the two stop agreeing the moment anyone seeks — scrubbing to the last + * ten seconds would otherwise report the whole track as played, and the + * play/skip rule would count it as a full listen. + */ + +export interface BankedListen { + /** When this listen began. Period keys come from here, not from now. */ + startedAt: Date; + /** Milliseconds of actual playback, excluding pauses and seeks. */ + msPlayed: number; +} + +export class ListenCycle { + private startedAt: Date | null = null; + private playedMs = 0; + private lastTickAt: number | null = null; + + /** Whether a listen is currently open and able to be banked. */ + get isOpen(): boolean { + return this.startedAt !== null; + } + + /** Playback accumulated in the current cycle, for the rewind check. */ + get msPlayedInCycle(): number { + return this.playedMs; + } + + /** + * Begin a listen. `at` is when it started, which is not always now — but is + * for every caller so far. + */ + open(at: Date = new Date()): void { + this.startedAt = at; + this.playedMs = 0; + this.lastTickAt = null; + } + + /** + * Fold the time since the previous tick into the total. + * + * `playing` false still folds in the elapsed interval, then stops the clock: + * the time between the last tick and the pause was really played. + */ + tick(playing: boolean, now: number = Date.now()): void { + this.accumulate(now); + if (playing) this.lastTickAt = now; + } + + /** + * Bank the listen and close the cycle. + * + * Null when there is nothing worth reporting — no cycle was open, or no + * playback accumulated. A zero-length listen is not a skip, it is a + * non-event, and writing one would put noise in `play_events`. + */ + close(now: number = Date.now()): BankedListen | null { + this.accumulate(now); + + const startedAt = this.startedAt; + const msPlayed = Math.round(this.playedMs); + + this.startedAt = null; + this.playedMs = 0; + this.lastTickAt = null; + + if (startedAt === null || msPlayed <= 0) return null; + return { startedAt, msPlayed }; + } + + /** + * Bank the listen and immediately open the next one, same track still loaded. + * + * The distinction from `close` is the whole fix: a looped or restarted track + * keeps playing, so a cycle must be open to receive it. `startedAt` becomes + * now rather than null, which also puts the two halves of a loop that crosses + * midnight into the right days. + */ + restart(now: number = Date.now()): BankedListen | null { + const banked = this.close(now); + this.open(new Date(now)); + return banked; + } + + private accumulate(now: number): void { + if (this.lastTickAt === null) return; + this.playedMs += now - this.lastTickAt; + this.lastTickAt = null; + } +} diff --git a/src/services/stats/rollups.test.ts b/src/services/stats/rollups.test.ts index 6f21dc4..367ec78 100644 --- a/src/services/stats/rollups.test.ts +++ b/src/services/stats/rollups.test.ts @@ -4,6 +4,7 @@ import { foldDeltas, rollupDeltas, rollupKey, + UNKNOWN_ENTITY_ID, type ListenSubject, type RollupDelta, } from './rollups'; @@ -69,8 +70,7 @@ function makeEvents(count: number, seed = 1): Event[] { return { subject: { trackId: 1 + next(40), - // Nulls on purpose: untagged files are common and must not become - // entity 0 or be silently counted against some other artist. + // Nulls on purpose: untagged files share the reserved fallback row. artistId: next(5) === 0 ? null : 1 + next(12), albumId: next(7) === 0 ? null : 1 + next(9), playlistId: next(3) === 0 ? 1 + next(4) : null, @@ -139,7 +139,7 @@ describe('rollupDeltas', () => { ); }); - it('skips null entities rather than counting them as id 0', () => { + it('groups null artist and album ids under the reserved fallback row', () => { const deltas = rollupDeltas({ subject: { trackId: 1, artistId: null, albumId: null, playlistId: null }, keys, @@ -147,8 +147,13 @@ describe('rollupDeltas', () => { countsAsPlay: true, }); - expect(deltas).toHaveLength(3); - expect(deltas.every((d) => d.entityType === 'track')).toBe(true); + expect(deltas).toHaveLength(9); + expect(deltas.filter((d) => d.entityType === 'artist')).toEqual( + expect.arrayContaining([expect.objectContaining({ entityId: UNKNOWN_ENTITY_ID })]), + ); + expect(deltas.filter((d) => d.entityType === 'album')).toEqual( + expect.arrayContaining([expect.objectContaining({ entityId: UNKNOWN_ENTITY_ID })]), + ); }); it('records milliseconds but no play for a skip or partial', () => { diff --git a/src/services/stats/rollups.ts b/src/services/stats/rollups.ts index 4b8b978..c086eb4 100644 --- a/src/services/stats/rollups.ts +++ b/src/services/stats/rollups.ts @@ -16,6 +16,15 @@ export type PeriodType = (typeof PERIOD_TYPES)[number]; export const ENTITY_TYPES = ['track', 'artist', 'album', 'playlist'] as const; export type EntityType = (typeof ENTITY_TYPES)[number]; +/** + * Reserved rollup id for an artist or album absent from a file's metadata. + * + * SQLite auto-increment ids begin at 1, so 0 cannot collide with a real row. + * Keeping the fallback out of the content tables also keeps its display name + * localised at render time rather than persisting one language in user data. + */ +export const UNKNOWN_ENTITY_ID = 0; + /** One `(period, entity)` cell and the amounts to add to it. */ export interface RollupDelta { periodType: PeriodType; @@ -26,7 +35,7 @@ export interface RollupDelta { msPlayed: number; } -/** The entities one listen belongs to. Nulls are skipped, not defaulted. */ +/** The entities one listen belongs to. Null artist and album ids share id 0. */ export interface ListenSubject { trackId: number; artistId: number | null; @@ -50,8 +59,8 @@ export interface ListenContribution { /** * Fan one listen out into every cell it touches. * - * Three periods times up to four entities, minus whatever is null. Written as - * a product rather than by hand so a new period or entity type cannot be added + * Three periods times four entities, minus a missing playlist. Written as a + * product rather than by hand so a new period or entity type cannot be added * to one and forgotten in the others. */ export function rollupDeltas({ @@ -68,13 +77,9 @@ export function rollupDeltas({ const entities: { entityType: EntityType; entityId: number }[] = [ { entityType: 'track', entityId: subject.trackId }, + { entityType: 'artist', entityId: subject.artistId ?? UNKNOWN_ENTITY_ID }, + { entityType: 'album', entityId: subject.albumId ?? UNKNOWN_ENTITY_ID }, ]; - if (subject.artistId !== null) { - entities.push({ entityType: 'artist', entityId: subject.artistId }); - } - if (subject.albumId !== null) { - entities.push({ entityType: 'album', entityId: subject.albumId }); - } if (subject.playlistId != null) { entities.push({ entityType: 'playlist', entityId: subject.playlistId }); }