fix(web): #426 classify in-queue track end as play_ended, not skip

The events dispatcher closed every prior open row as play_skipped on
any track-id change. Server-side RecordPlaySkipped force-sets
was_skipped=true regardless of completion, so a queue played start
to finish reported tracks 1..N-1 as skips — inflating
recommendation skip-ratios (-skipRatio*SkipPenalty) and degrading
For You / Discover quality for web listeners.

Now: on track change, close the prior row as play_ended if that
track reached ~its duration (3s tolerance, matching the Flutter
PlayEventsReporter), else play_skipped at the real last position.

Race fix: the store synchronously resets position/duration to 0 on
track change, so reading lastPositionMs at change-time would see 0
and misclassify. Track per-open-row state (openReachedEnd,
openLastPositionMs, openDurationMs) updated ONLY while the open
track is current — a track change can't clobber them before the
close branch runs.

Brings web wire behavior back in line with Flutter. Test added:
auto-advance after reaching duration → play_ended, never skipped;
existing mid-track-skip and pause-at-end tests still hold.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

web/src/lib/player/events.svelte.test.ts

@@ -92,6 +92,35 @@ describe('useEventsDispatcher', () => {
     cleanup();
   });
 
+  test('auto-advance after reaching duration POSTs play_ended, not skip (#426)', async () => {
+    const cleanup = $effect.root(() => useEventsDispatcher());
+
+    playQueue([track('1', 200), track('2', 200)]);
+    flushSync();
+    reportDuration(200);
+    reportStateFromAudio('playing');
+    flushSync();
+    await Promise.resolve();
+    await Promise.resolve();
+    // Track 1 plays through to its end...
+    reportTimeUpdate(200);
+    flushSync();
+    // ...then the queue advances to track 2 (track id changes, store
+    // resets position/duration to 0 synchronously).
+    skipNext();
+    flushSync();
+    await Promise.resolve();
+
+    const closesForPe1 = (api.post as ReturnType<typeof vi.fn>).mock.calls
+      .map((c) => c[1] as { type: string; play_event_id?: string })
+      .filter((b) => b.play_event_id === 'pe-1');
+    // The finished track must close as ended, never skipped.
+    expect(closesForPe1.some((b) => b.type === 'play_ended')).toBe(true);
+    expect(closesForPe1.some((b) => b.type === 'play_skipped')).toBe(false);
+
+    cleanup();
+  });
+
   test('user-initiated skip mid-track POSTs play_skipped', async () => {
     const cleanup = $effect.root(() => useEventsDispatcher());
 

web/src/lib/player/events.svelte.ts

@@ -9,21 +9,49 @@ import type { PlayStartedResponse, EventOkResponse } from '$lib/api/types';
 // The dispatcher is a small state machine over (current track id, state).
 // Per spec data-flow:
 //   - First transition into 'playing' for a new track → POST play_started.
-//   - Track id changes while a play is open → POST play_skipped on the prior.
+//   - 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.
 //   - Natural completion (state moves from 'playing' to 'paused' AND
-//     position >= duration > 0) → POST play_ended on the open row.
+//     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;
+
 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.
   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();
 
-  // Track the latest position for use by close events. Cheap; no API call.
   $effect(() => {
-    lastPositionMs = Math.round(player.position * 1000);
+    const posMs = Math.round(player.position * 1000);
+    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;
+        }
+      }
+    }
   });
 
   // Main state machine. Reacts only to (current track id, state) transitions.
@@ -32,17 +60,25 @@ export function useEventsDispatcher(): void {
     const s = player.state;
     const tid = t?.id ?? null;
 
-    // Track changed: close any prior open play as a skip.
+    // 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).
     if (tid !== prevTrackId && openPlayEventId) {
       const id = openPlayEventId;
-      const pos = lastPositionMs;
+      const finished = openReachedEnd;
+      const endedMs = openDurationMs > 0 ? openDurationMs : openLastPositionMs;
+      const skipMs = openLastPositionMs;
       openPlayEventId = null;
       openTrackId = null;
-      api.post<EventOkResponse>('/api/events', {
-        type: 'play_skipped',
-        play_event_id: id,
-        position_ms: pos
-      }).catch(() => {});
+      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(() => {});
     }
 
     // Entered playing for a new track (no open row yet for this track).
@@ -62,6 +98,9 @@ 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,