Fix/005 redgifs playback

frontend/src/components/VideoPlayer.jsx

@@ -3,6 +3,7 @@ import { forwardRef, useEffect, useImperativeHandle, useMemo, useRef, useState }
 
 const isDev = import.meta.env.DEV;
 const DIRECT_LOAD_TIMEOUT_MS = 5000;
+const EMPTY_COMPANION_AUDIO_URLS = [];
 
 function logSourceInputs({ mp4Url, hlsUrl, dashUrl, hasAudio, sourceKind }) {
   if (!isDev) return;
@@ -25,7 +26,7 @@ const VideoPlayer = forwardRef(function VideoPlayer(
     mp4Url,
     hlsUrl,
     dashUrl,
-    companionAudioUrls = [],
+    companionAudioUrls = EMPTY_COMPANION_AUDIO_URLS,
     hasAudio = null,
     sourceKind = null,
     posterUrl = '',

issues/005-redgifs-playback-loop.md

@@ -4,6 +4,50 @@
 
 None. Standalone bug, parked from session work on 2026-05-08.
 
+## 2026-05-26 evidence (root cause identified)
+
+Re-investigated under branch `fix/005-redgifs-playback` with temporary instrumentation on the 17-dep `useEffect` in `frontend/src/components/VideoPlayer.jsx`. The instrumentation logs an effect run counter and which deps changed between consecutive fires.
+
+**Observed:**
+
+- The loop fires at roughly 1500 effect runs per second (~7900 fires in a 5-second capture).
+- `loadedmetadata` DOES fire (duration appears in the native controls bar), contra the original "never reaches `loadedmetadata`" assumption above. The loop is downstream of metadata, not upstream.
+- The **only** dep that changes between fires is `sanitizedCompanionAudioUrls`. All 16 other deps (including `preferredMuted`, `onDiagnostics`, `mp4Url`, `sourceKind`, etc.) are stable.
+
+**Cross-reference against the original four hypotheses:**
+
+- Hypothesis #1 (`preferredMuted` feedback loop via `volumechange` → `onMutedChange` → parent `setPersistMuted`): **eliminated**. `preferredMuted` never appears in the changed-dep set.
+- Hypothesis #2 (inline-computed prop with new ref per render): **partially right direction, wrong location**. The fresh literal is not in the parent's JSX; it's the default value in VideoPlayer's own destructure.
+- Hypothesis #3 (`onDiagnostics` instability): **eliminated**. LightboxModal does not pass `onDiagnostics`, and the dep is stable in the trace.
+- Hypothesis #4 (`current` reference instability): **eliminated**. Other deps tied to `current` (`mp4Url`, `sourceKind`, `posterUrl`) are stable in the trace.
+
+**Actual root cause:**
+
+`frontend/src/components/VideoPlayer.jsx:28` destructures the prop with a default value:
+
+```js
+companionAudioUrls = [],
+```
+
+JavaScript evaluates default-expression `[]` fresh on every function invocation, so when a caller omits `companionAudioUrls`, the destructure produces a brand-new array reference on every render of VideoPlayer.
+
+`frontend/src/components/LightboxModal.jsx:596` — the RedGIFs embed `<VideoPlayer>` call site — does **not** pass `companionAudioUrls`. The regular video call site at LightboxModal.jsx:576-591 does pass it (as a memoized prop on lines 157-160), which is why the v.redd.it HLS path does not loop.
+
+The unstable `companionAudioUrls` reference flows into `useMemo` (VideoPlayer.jsx:55-58):
+
+```js
+const sanitizedCompanionAudioUrls = useMemo(
+  () => (Array.isArray(companionAudioUrls) ? companionAudioUrls.filter(Boolean) : []),
+  [companionAudioUrls]
+);
+```
+
+React's `Object.is` check on `[companionAudioUrls]` fails every render, the memo factory re-runs, and `.filter(Boolean)` produces yet another fresh array. `sanitizedCompanionAudioUrls` is therefore a new reference every render, which invalidates the 17-dep `useEffect`. Cleanup runs `video.removeAttribute('src')` + `video.load()`, events fire, `setVideoMetrics` / diagnostics setState trigger a re-render, and the loop closes.
+
+The same path is also active for the prebuffer `<VideoPlayer prebufferOnly />` at LightboxModal.jsx:647, which also omits `companionAudioUrls`. The fix below resolves both surfaces at once.
+
+This is a **fifth hypothesis** — destructure-default-array-literal — that was not on the original list.
+
 ## What's broken
 
 When a RedGIFs-embed post is opened in the lightbox, the `<video>` element makes hundreds of media requests to `/api/external/redgifs/:id/stream`, each one cancelled within 5–16ms. The video shows a loading spinner and never reaches `loadedmetadata`. Confirmed reproducible in Chrome on r/just18, r/collegesluts and similar NSFW subreddits with RedGIFs links.

memory.md

@@ -42,6 +42,22 @@ This file is for the human + the AI assistant. Keep entries dated. Newest at top
 
 Append-only. Newest at top.
 
+### 2026-05-26: Pattern — destructure-default-non-primitive-literal causes useMemo/useEffect dependency invalidation (issue 005)
+
+When a React component destructures a prop with a default value that is a non-primitive literal (`= []`, `= {}`, `= () => {}`, etc.), JavaScript evaluates that default expression on every function invocation. If the caller omits the prop, the destructure produces a brand-new reference every render. If that destructured value then feeds into a `useMemo` or `useEffect` dependency array, the consumer's `Object.is` check fails on every render and the memo factory or effect re-runs even though the semantic value is unchanged. Downstream `setState` calls in event handlers (e.g., `video.load()` firing `loadedmetadata` → `setVideoMetrics`) can close the loop and produce a runaway render cycle.
+
+**Caught in the wild as issue 005:** `frontend/src/components/VideoPlayer.jsx:28` had `companionAudioUrls = []`. `LightboxModal.jsx`'s RedGIFs embed branch and prebuffer branch both omitted the prop. The default produced a fresh `[]` per render. The downstream `useMemo` invalidated, `sanitizedCompanionAudioUrls` became a new reference each render, the 17-dep `useEffect` re-fired ~1500x/sec, and `video.load()` in the cleanup spammed cancelled requests at `/api/external/redgifs/:id/stream`. The regular `v.redd.it` HLS path was unaffected because `LightboxModal` did pass a memoised `companionAudioUrls` to that VideoPlayer.
+
+**Fix pattern:** hoist the default to a module-level `const`, so the default expression resolves to a single shared reference across all invocations. Example: `const EMPTY = []; ... function Foo({ items = EMPTY }) { ... }`.
+
+**Triage pattern when a similar render loop appears:**
+1. Add temporary instrumentation to the suspect `useEffect`: an effect-run counter plus an `Object.is`-based dep comparator that logs which deps changed between fires (see commit `6b4a0e9` for the canonical implementation). Tag the logs with a distinctive prefix so the console filter can isolate them.
+2. Reproduce the loop. Capture ~5s of console output. The unstable dep will be the one that appears in every "changed deps" entry.
+3. Trace the unstable dep back through any `useMemo` chain to its origin. If the origin is a destructured prop with a non-primitive default literal, you've hit this pattern.
+4. Hoist the default. Remove the instrumentation in a separate cleanup commit so the fix diff is readable on its own.
+
+**Grep this entry by `destructure-default-non-primitive` when triaging similar loops in the future.**
+
 ### 2026-05-26: Tooling scaffold (issue 010)
 - **Warn-baseline for ESLint on legacy code.** Rules existing code violates are downgraded to `warn` at the config level rather than refactored. Tightening to `error` belongs in a dedicated cleanup task, not 010. Specific downgrades: `no-empty` (allowEmptyCatch), `react-hooks/set-state-in-effect`, `react-hooks/immutability`, `react/no-unescaped-entities`, `no-constant-binary-expression`.
 - **`no-dupe-keys` stays strict.** The one duplicate `width` key in `VideoPlayer.jsx` was deleted by exception (one line, behavior-neutral, user-approved). Future dupe-key bugs will still be caught.