Skip to content

Report a play when the track is done with, not at the halfway mark #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeffcrouse merged 1 commit into from
Aug 1, 2026
Merged

Report a play when the track is done with, not at the halfway mark #57

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 104 additions & 16 deletions packages/frontend/src/hooks/__tests__/usePlayTracking.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ describe('usePlayTracking', () => {
expect(plays).toHaveLength(0);
});

it('records a play once the threshold is reached', async () => {
it('records a play when the track is done with, not partway through', async () => {
const track = createMockTrack('track-2');
const next = createMockTrack('track-2b');
const { rerender } = renderHook(() => usePlayTracking());

act(() => {
Expand All @@ -141,6 +142,15 @@ describe('usePlayTracking', () => {
usePlayerStore.setState({ currentTime: 95 });
});
rerender();
await flush();

// Past half the track, which used to be enough to send it. Nothing goes yet.
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(0);

act(() => {
usePlayerStore.setState({ currentTrack: next, currentTime: 0, _advanceReason: 'ended' });
});
rerender();

await waitFor(() => {
expect(mockDeliver).toHaveBeenCalledWith(
Expand All @@ -152,6 +162,42 @@ describe('usePlayTracking', () => {
}, { timeout: 2000 });
});

/**
* The defect this hook's note describes, pinned.
*
* Delivering at the halfway mark froze `completion_ratio` at ~0.5 for every web play
* regardless of how much was really heard — 289 of 357 completed rows on the live
* database sat in the 0.5–0.6 bucket. Completion is what ADR-0005 ranks on, so a
* constant made the whole signal worthless. A track heard almost to the end must
* report almost 1.
*/
it('reports the ratio of the whole listen, not the moment it passed half', async () => {
const track = createMockTrack('track-full');
const next = createMockTrack('track-full-b');
const { rerender } = renderHook(() => usePlayTracking());

act(() => {
usePlayerStore.setState({ currentTrack: track, isPlaying: true, duration: 180, currentTime: 0 });
});
rerender();
// Straight past the old halfway trigger and on to the end of the track.
act(() => { usePlayerStore.setState({ currentTime: 90 }); });
rerender();
act(() => { usePlayerStore.setState({ currentTime: 178 }); });
rerender();

act(() => {
usePlayerStore.setState({ currentTrack: next, currentTime: 0, _advanceReason: 'ended' });
});
rerender();

await waitFor(() => {
const play = mockDeliver.mock.calls.find((c) => c[0] === 'track-full' && c[1] === 'played');
expect(play).toBeDefined();
expect((play![2] as { completion_ratio?: number }).completion_ratio).toBeCloseTo(0.99, 2);
}, { timeout: 2000 });
});

it('does not accumulate time while paused', async () => {
const track = createMockTrack('track-3');
const { rerender } = renderHook(() => usePlayTracking());
Expand Down Expand Up @@ -299,22 +345,20 @@ describe('usePlayTracking', () => {
const b = { ...createMockTrack('track-b'), duration_seconds: 374 };
const { rerender } = renderHook(() => usePlayTracking());

// A plays past its threshold and is recorded.
// A plays, then the crossfade advances to B — which is where A is recorded.
act(() => {
usePlayerStore.setState({ currentTrack: a, isPlaying: true, duration: 75, currentTime: 0 });
});
rerender();
act(() => { usePlayerStore.setState({ currentTime: 40 }); });
rerender();
await waitFor(() => {
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(1);
});

// Crossfade advances to B...
act(() => {
usePlayerStore.setState({ currentTrack: b, currentTime: 0, _advanceReason: 'crossfade' });
});
rerender();
await waitFor(() => {
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(1);
});
// ...then fails, rolling straight back to A having played none of B.
act(() => {
usePlayerStore.setState({ currentTrack: a, currentTime: 40, _advanceReason: 'error' });
Expand All @@ -334,42 +378,86 @@ describe('usePlayTracking', () => {
it('still records a genuine replay after another track was listened to', async () => {
const a = createMockTrack('track-a');
const b = createMockTrack('track-b');
const c = createMockTrack('track-c');
const { rerender } = renderHook(() => usePlayTracking());

// A plays, then B — which records A.
act(() => {
usePlayerStore.setState({ currentTrack: a, isPlaying: true, duration: 180, currentTime: 0 });
});
rerender();
act(() => { usePlayerStore.setState({ currentTime: 95 }); });
rerender();
act(() => {
usePlayerStore.setState({ currentTrack: b, currentTime: 0, _advanceReason: 'ended' });
});
rerender();
await waitFor(() => {
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(1);
});

// B genuinely plays through.
// B genuinely plays through, then back to A — which records B and, because a
// different track was really listened to, frees A to be recorded again.
act(() => { usePlayerStore.setState({ currentTime: 95 }); });
rerender();
act(() => {
usePlayerStore.setState({ currentTrack: b, currentTime: 0, _advanceReason: 'ended' });
usePlayerStore.setState({ currentTrack: a, currentTime: 0, _advanceReason: 'user' });
});
rerender();
act(() => { usePlayerStore.setState({ currentTime: 95 }); });
rerender();
await waitFor(() => {
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(2);
});

// Returning to A is a real replay and must count.
// The replay of A is a real listen and must count on the way out of it.
act(() => { usePlayerStore.setState({ currentTime: 95 }); });
rerender();
act(() => {
usePlayerStore.setState({ currentTrack: a, currentTime: 0, _advanceReason: 'user' });
usePlayerStore.setState({ currentTrack: c, currentTime: 0, _advanceReason: 'ended' });
});
rerender();
act(() => { usePlayerStore.setState({ currentTime: 95 }); });
rerender();

await waitFor(() => {
const plays = mockDeliver.mock.calls.filter((c) => c[1] === 'played' && c[0] === 'track-a');
const plays = mockDeliver.mock.calls.filter((call) => call[1] === 'played' && call[0] === 'track-a');
expect(plays).toHaveLength(2);
});
});

/**
* The durability the old halfway delivery was quietly providing: close the tab
* mid-track and the play still counted, because it had already been sent. Reporting
* at the end would have lost that silently, so it is restored here — best effort, but
* with the ratio that was actually heard rather than a frozen one.
*/
it('reports the track in progress when the page goes away', async () => {
const track = createMockTrack('track-hide');
const { rerender } = renderHook(() => usePlayTracking());

act(() => {
usePlayerStore.setState({ currentTrack: track, isPlaying: true, duration: 180, currentTime: 0 });
});
rerender();
act(() => { usePlayerStore.setState({ currentTime: 120 }); });
rerender();
await flush();
expect(mockDeliver.mock.calls.filter((c) => c[1] === 'played')).toHaveLength(0);

act(() => { window.dispatchEvent(new Event('pagehide')); });
await flush();

const play = mockDeliver.mock.calls.find((c) => c[0] === 'track-hide' && c[1] === 'played');
expect(play).toBeDefined();
expect((play![2] as { completion_ratio?: number }).completion_ratio).toBeCloseTo(0.667, 2);

// And it must not be reported a second time if the page survives and the track
// is later advanced past.
act(() => {
usePlayerStore.setState({ currentTrack: createMockTrack('track-after'), currentTime: 0, _advanceReason: 'ended' });
});
rerender();
await flush();

expect(mockDeliver.mock.calls.filter((c) => c[0] === 'track-hide' && c[1] === 'played')).toHaveLength(1);
});
});

describe('context', () => {
Expand Down
103 changes: 79 additions & 24 deletions packages/frontend/src/hooks/usePlayTracking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const log = createLogger('PlayTracking');

/** Reaching the scrobble threshold is what counts as a play. */
const MIN_PLAY_SECONDS = 30;
const MAX_PLAY_SECONDS = 4 * 60;

/**
* Map the client's advance reason onto the backend's `StopReason` (ADR-0004).
Expand All @@ -33,6 +32,21 @@ function toStopReason(reason: AdvanceReason): ListenStopReason | null {
}
}

/**
* The share of the track that was heard, or undefined when the duration is unknown.
*
* Clamped, because the two figures come from different places: played time is accumulated
* from the engine's clock while the duration is track metadata, and a track that loops or
* whose tag understates its length can produce more played seconds than the track is long.
* The server clamps too, but sending 1.1 makes every stored figure a question about which
* end did the clamping — and the native client (`PlaybackReport.completionRatio`) has
* always clamped, so this is the two clients agreeing rather than a new rule.
*/
function completionRatio(playedSeconds: number, durationSeconds: number): number | undefined {
if (!durationSeconds) return undefined;
return Math.min(Math.max(playedSeconds / durationSeconds, 0), 1);
}

/** Queue source types line up with the backend's PlayContext, apart from 'library'. */
function toContext(sourceType: string | undefined): ListenContext | undefined {
if (!sourceType) return undefined;
Expand All @@ -46,15 +60,29 @@ function toContext(sourceType: string | undefined): ListenContext | undefined {
* Track listening and report it to the backend.
*
* Two things are recorded:
* - a **play**, on the Last.fm rule — ≥30s listened AND ≥min(half the track, 4 min).
* This bumps ProfilePlayHistory and is unchanged from before.
* - a **play**, once ≥30s has been listened to. This bumps ProfilePlayHistory.
* - a **listening event** for every track the listener moves on from, including short
* skips, which previously produced no API call at all. This is the negative signal
* ADR-0005's radio needs, and it only accumulates in real time.
*
* The outcome is derived server-side from the stop reason plus completion ratio, so a
* crossfade (which advances early, around 0.9) is not mistaken for a skip and a failed
* load is never mistaken for dislike.
*
* **Reported when the track is done with, not partway through — and that is the whole
* point.** This used to deliver the play the moment listening crossed
* `min(duration / 2, 4 min)`, sending `completion_ratio` as measured *at that instant*.
* Nothing ever revised it, so every web play landed with a ratio of almost exactly 0.5
* whether the listener heard half the track or all of it. Measured on the live database
* on 2026-08-01: **289 of 357 completed events sat in the 0.5–0.6 bucket**, against native
* client rows correctly reading 0.95–1.00. Completion is the taste signal ADR-0005 ranks
* on, and a constant carries none — so a month of "accumulating data" would have been a
* month of noise.
*
* Nothing changes about *which* plays count. The early delivery only decided *when*: a
* play short of the half mark but past 30s was already reported on track change, by the
* same rule that now reports all of them. The Last.fm half/4-minute threshold turned out
* to gate nothing, which is why it is gone rather than merely moved.
*/
export function usePlayTracking() {
const { currentTrack, currentTime, duration, isPlaying } = usePlayerStore(
Expand Down Expand Up @@ -100,14 +128,16 @@ export function usePlayTracking() {
// Already counted as a play; nothing further to say about it.
log.debug('outgoing track already recorded as a play', { previousId });
} else if (playedSeconds >= MIN_PLAY_SECONDS) {
// Partial play past the minimum but short of the scrobble threshold. Counts
// toward the aggregate, exactly as it did before.
// A play, reported now that the final figure is known. Marked as recorded for the
// same reason the threshold delivery used to be: so a crossfade rollback cannot
// report the same listen twice.
recordedTrackRef.current = previousId;
void deliverListenEvent(
previousId,
'played',
{
track_duration: outgoingDuration || undefined,
completion_ratio: outgoingDuration ? playedSeconds / outgoingDuration : undefined,
completion_ratio: completionRatio(playedSeconds, outgoingDuration),
context,
},
playedSeconds,
Expand Down Expand Up @@ -175,30 +205,55 @@ export function usePlayTracking() {
recordedTrackRef.current = null;
}

const recordThreshold = Math.min(duration / 2, MAX_PLAY_SECONDS);
// Nothing is delivered here. The play is reported when the track is done with, where
// the completion ratio is final — see this hook's own note on why.
// eslint-disable-next-line react-hooks/exhaustive-deps -- Only re-run when track ID changes, not object reference
}, [currentTrack?.id, currentTime, duration, isPlaying]);

if (accumulatedTimeRef.current < MIN_PLAY_SECONDS) return;
// Report the track in progress if the page goes away before it is finished with.
//
// The cost of reporting at the end rather than partway through: closing the tab mid-track
// used to still count the play, because it had already been sent at the halfway mark.
// This restores that without restoring the frozen ratio — the figures here are whatever
// has actually been heard.
//
// Best effort, and honestly so. `pagehide` gives the request a chance to leave and the
// IndexedDB fallback a chance to catch it, but a browser tearing the page down owes
// neither of them anything. `visibilitychange` is the one that actually fires on mobile,
// where tabs are discarded without warning; marking the track recorded keeps a later
// track change from reporting the same listen twice if the page survives after all.
useEffect(() => {
const flush = () => {
const trackId = currentTrackIdRef.current;
const playedSeconds = accumulatedTimeRef.current;
if (!trackId || playedSeconds < MIN_PLAY_SECONDS) return;
if (recordedTrackRef.current === trackId) return;

if (accumulatedTimeRef.current >= recordThreshold) {
recordedTrackRef.current = currentTrack.id;
const played = accumulatedTimeRef.current;
const outgoingDuration = lastDurationRef.current;
const { queueSource } = usePlayerStore.getState();
recordedTrackRef.current = trackId;

deliverListenEvent(
currentTrack.id,
void deliverListenEvent(
trackId,
'played',
{
track_duration: duration,
completion_ratio: duration ? played / duration : undefined,
track_duration: outgoingDuration || undefined,
completion_ratio: completionRatio(playedSeconds, outgoingDuration),
context: toContext(queueSource?.type),
},
played,
).catch((err) => {
// Reset so we can retry on the next threshold check
log.error('Failed to record play:', err);
recordedTrackRef.current = null;
});
}
// eslint-disable-next-line react-hooks/exhaustive-deps -- Only re-run when track ID changes, not object reference
}, [currentTrack?.id, currentTime, duration, isPlaying]);
playedSeconds,
);
};

const onVisibilityChange = () => {
if (document.visibilityState === 'hidden') flush();
};

window.addEventListener('pagehide', flush);
document.addEventListener('visibilitychange', onVisibilityChange);
return () => {
window.removeEventListener('pagehide', flush);
document.removeEventListener('visibilitychange', onVisibilityChange);
};
}, []);
}
Loading