Web client now always POSTs play_ended with the actual position
played to, matching the Android fix at d82e744d (Scribe 519). The
server's skip rule (skip_max_completion_ratio +
skip_max_duration_played_ms, default 50%/30s) classifies was_skipped
from the duration_played_ms; the dispatcher no longer makes the
call itself.
Three transitions all collapse to play_ended:
- Track change (was: reachedEnd ? play_ended : play_skipped)
- Natural end via paused-at-duration (was already play_ended)
- pagehide via sendBeacon (was: play_skipped)
Drops the openReachedEnd field, COMPLETION_TOLERANCE_MS constant,
and the position-vs-duration threshold check that backed the prior
classification. play_skipped wire endpoint stays in place for a
future explicit-dislike affordance.
Two test cases updated:
- "user-initiated next mid-track" now asserts play_ended with
duration_played_ms = the partial position, not play_skipped.
- "pagehide fires sendBeacon" now parses the beacon payload and
asserts type=play_ended + duration_played_ms = last position.
Flutter NOT touched — deprecated per M8.
This commit is contained in:
@@ -121,27 +121,40 @@ describe('useEventsDispatcher', () => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
test('user-initiated skip mid-track POSTs play_skipped', async () => {
|
||||
test('user-initiated next mid-track POSTs play_ended with the actual position', async () => {
|
||||
// Under the post-d82e744d shape the client always sends play_ended;
|
||||
// the server's skip rule (skip_max_completion_ratio +
|
||||
// skip_max_duration_played_ms) decides was_skipped from the
|
||||
// duration_played_ms. play_skipped is reserved for a future
|
||||
// explicit-dislike affordance and isn't called from track-change.
|
||||
const cleanup = $effect.root(() => useEventsDispatcher());
|
||||
|
||||
playQueue([track('1'), track('2')]);
|
||||
flushSync();
|
||||
reportDuration(200);
|
||||
reportStateFromAudio('playing');
|
||||
flushSync();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
// Played 40 seconds in, then user hit next.
|
||||
reportTimeUpdate(40);
|
||||
flushSync();
|
||||
skipNext();
|
||||
flushSync();
|
||||
await Promise.resolve();
|
||||
|
||||
const calls = (api.post as ReturnType<typeof vi.fn>).mock.calls;
|
||||
const types = calls.map((c) => (c[1] as { type: string }).type);
|
||||
expect(types).toContain('play_skipped');
|
||||
const closesForPe1 = (api.post as ReturnType<typeof vi.fn>).mock.calls
|
||||
.map((c) => c[1] as { type: string; play_event_id?: string; duration_played_ms?: number })
|
||||
.filter((b) => b.play_event_id === 'pe-1');
|
||||
expect(closesForPe1.some((b) => b.type === 'play_ended')).toBe(true);
|
||||
expect(closesForPe1.some((b) => b.type === 'play_skipped')).toBe(false);
|
||||
const ended = closesForPe1.find((b) => b.type === 'play_ended');
|
||||
expect(ended?.duration_played_ms).toBe(40000);
|
||||
|
||||
cleanup();
|
||||
});
|
||||
|
||||
test('pagehide fires sendBeacon when a play is open', async () => {
|
||||
test('pagehide fires sendBeacon with play_ended carrying last position', async () => {
|
||||
// jsdom doesn't ship navigator.sendBeacon — install a stub before spy.
|
||||
if (typeof navigator.sendBeacon !== 'function') {
|
||||
Object.defineProperty(navigator, 'sendBeacon', {
|
||||
@@ -152,17 +165,25 @@ describe('useEventsDispatcher', () => {
|
||||
const cleanup = $effect.root(() => useEventsDispatcher());
|
||||
const beacon = vi.spyOn(navigator, 'sendBeacon').mockReturnValue(true);
|
||||
|
||||
playQueue([track('1')]);
|
||||
playQueue([track('1', 200)]);
|
||||
flushSync();
|
||||
reportDuration(200);
|
||||
reportStateFromAudio('playing');
|
||||
flushSync();
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
reportTimeUpdate(73);
|
||||
flushSync();
|
||||
|
||||
window.dispatchEvent(new Event('pagehide'));
|
||||
|
||||
expect(beacon).toHaveBeenCalled();
|
||||
expect(beacon.mock.calls[0][0]).toBe('/api/events');
|
||||
const blob = beacon.mock.calls[0][1] as Blob;
|
||||
const text = await blob.text();
|
||||
const payload = JSON.parse(text) as { type: string; duration_played_ms: number };
|
||||
expect(payload.type).toBe('play_ended');
|
||||
expect(payload.duration_played_ms).toBe(73000);
|
||||
|
||||
cleanup();
|
||||
beacon.mockRestore();
|
||||
|
||||
@@ -6,36 +6,34 @@ import type { PlayStartedResponse, EventOkResponse } from '$lib/api/types';
|
||||
// useEventsDispatcher installs $effect-based watchers that emit play events
|
||||
// to /api/events. Call once from +layout.svelte's mount.
|
||||
//
|
||||
// The dispatcher is a small state machine over (current track id, state).
|
||||
// Per spec data-flow:
|
||||
// The dispatcher is a small state machine over (current track id, state):
|
||||
// - First transition into 'playing' for a new track → POST play_started.
|
||||
// - Track id changes while a play is open → close the prior row:
|
||||
// play_ended if it reached ~its duration, else play_skipped (#426).
|
||||
// Without this every auto-advanced in-queue track was reported as a
|
||||
// skip, force-flagging was_skipped=true server-side and poisoning
|
||||
// the recommendation skip-ratio. Mirrors the Flutter reporter.
|
||||
// - Track id changes while a play is open → close the prior row with
|
||||
// play_ended carrying the actual position played to. Server applies
|
||||
// its skip rule (skip_max_completion_ratio + skip_max_duration_played_ms,
|
||||
// default 50%/30s) and decides was_skipped from that.
|
||||
// - Natural completion (state moves from 'playing' to 'paused' AND
|
||||
// position >= duration > 0) → POST play_ended on the open row
|
||||
// (covers the last track / explicit pause-at-end).
|
||||
// - pagehide with a live row → navigator.sendBeacon a play_skipped.
|
||||
|
||||
// Within this much of the known duration counts as "finished", not a
|
||||
// skip. Matches the Flutter PlayEventsReporter tolerance.
|
||||
const COMPLETION_TOLERANCE_MS = 3000;
|
||||
// position >= duration > 0) → POST play_ended on the open row.
|
||||
// - pagehide with a live row → navigator.sendBeacon a play_ended with
|
||||
// the last position. The server's rule still classifies skip-vs-not.
|
||||
//
|
||||
// Earlier shape made the skip-vs-ended call client-side via a "within 3s
|
||||
// of duration" rule, which left the server's threshold dead code and
|
||||
// treated every "next" tap as a skip even after most of the track played.
|
||||
// Backflow of the Android fix (commit d82e744d, 2026-06-01); see Scribe
|
||||
// task 519. Flutter client is deprecated and not updated.
|
||||
|
||||
export function useEventsDispatcher(): void {
|
||||
let openPlayEventId: string | null = null;
|
||||
let openTrackId: string | null = null;
|
||||
// lastPositionMs tracks the *current* track's position (used by the
|
||||
// pagehide beacon). openLastPositionMs / openReachedEnd track the
|
||||
// *open row's* track specifically and are updated ONLY while that
|
||||
// track is current — so a track change (which synchronously resets
|
||||
// the store's position to 0 and duration to 0) can't clobber them
|
||||
// before the close branch reads them. This is the #426 race fix.
|
||||
// pagehide beacon). openLastPositionMs tracks the *open row's* track
|
||||
// specifically and is updated ONLY while that track is current — so a
|
||||
// track change (which synchronously resets the store's position to 0
|
||||
// and duration to 0) can't clobber it before the close branch reads
|
||||
// it. This is the #426 race fix.
|
||||
let lastPositionMs = 0;
|
||||
let openLastPositionMs = 0;
|
||||
let openDurationMs = 0;
|
||||
let openReachedEnd = false;
|
||||
let prevTrackId: string | null = null;
|
||||
let prevState: string | null = null;
|
||||
const clientId = getOrCreateClientId();
|
||||
@@ -45,12 +43,6 @@ export function useEventsDispatcher(): void {
|
||||
lastPositionMs = posMs;
|
||||
if (openTrackId && player.current?.id === openTrackId) {
|
||||
openLastPositionMs = posMs;
|
||||
if (player.duration > 0) {
|
||||
openDurationMs = Math.round(player.duration * 1000);
|
||||
if (posMs >= openDurationMs - COMPLETION_TOLERANCE_MS) {
|
||||
openReachedEnd = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
@@ -60,25 +52,20 @@ export function useEventsDispatcher(): void {
|
||||
const s = player.state;
|
||||
const tid = t?.id ?? null;
|
||||
|
||||
// Track changed: close the prior open row. If that track reached
|
||||
// ~its duration it finished naturally (auto-advance) → play_ended
|
||||
// with its full duration; otherwise the user moved on early →
|
||||
// play_skipped at the last position it actually reached (#426).
|
||||
// Track changed: close the prior open row. Always send play_ended
|
||||
// with the actual position played; the server's skip rule decides
|
||||
// whether to flag was_skipped.
|
||||
if (tid !== prevTrackId && openPlayEventId) {
|
||||
const id = openPlayEventId;
|
||||
const finished = openReachedEnd;
|
||||
const endedMs = openDurationMs > 0 ? openDurationMs : openLastPositionMs;
|
||||
const skipMs = openLastPositionMs;
|
||||
const dur = openLastPositionMs;
|
||||
openPlayEventId = null;
|
||||
openTrackId = null;
|
||||
openDurationMs = 0;
|
||||
openLastPositionMs = 0;
|
||||
openReachedEnd = false;
|
||||
api.post<EventOkResponse>('/api/events',
|
||||
finished
|
||||
? { type: 'play_ended', play_event_id: id, duration_played_ms: endedMs }
|
||||
: { type: 'play_skipped', play_event_id: id, position_ms: skipMs }
|
||||
).catch(() => {});
|
||||
api.post<EventOkResponse>('/api/events', {
|
||||
type: 'play_ended',
|
||||
play_event_id: id,
|
||||
duration_played_ms: dur
|
||||
}).catch(() => {});
|
||||
}
|
||||
|
||||
// Entered playing for a new track (no open row yet for this track).
|
||||
@@ -98,9 +85,7 @@ export function useEventsDispatcher(): void {
|
||||
const dur = lastPositionMs;
|
||||
openPlayEventId = null;
|
||||
openTrackId = null;
|
||||
openDurationMs = 0;
|
||||
openLastPositionMs = 0;
|
||||
openReachedEnd = false;
|
||||
api.post<EventOkResponse>('/api/events', {
|
||||
type: 'play_ended',
|
||||
play_event_id: id,
|
||||
@@ -112,14 +97,16 @@ export function useEventsDispatcher(): void {
|
||||
prevState = s;
|
||||
});
|
||||
|
||||
// Page close: sendBeacon a play_skipped if an open play exists.
|
||||
// Page close: sendBeacon a play_ended if an open play exists. The
|
||||
// server's skip rule decides classification from the duration we
|
||||
// actually played.
|
||||
if (typeof window !== 'undefined') {
|
||||
const onPageHide = () => {
|
||||
if (!openPlayEventId) return;
|
||||
const payload = JSON.stringify({
|
||||
type: 'play_skipped',
|
||||
type: 'play_ended',
|
||||
play_event_id: openPlayEventId,
|
||||
position_ms: lastPositionMs
|
||||
duration_played_ms: lastPositionMs
|
||||
});
|
||||
navigator.sendBeacon('/api/events', new Blob([payload], { type: 'application/json' }));
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user