From 5cecefc924978c3ad9dba7a9757be901ed6f5ba3 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Sun, 17 May 2026 05:34:19 -0700 Subject: [PATCH 1/3] chore(quality): batch dedup pass across server + client Closes 20 items from PLAN.md "Code quality / dedup": Server helpers - `runFfmpegProcess` extracted in `server/lib/ffmpeg.js`, adopted by audioMux, upscale, faststart, generateThumbnail, extractEvaluationFrames - `sanitizeListWith` in `storyBible.js` (3 sites consolidated) - `listDirectoryByExtension` in `fileUtils.js` (3 sites) - `mockPathsDataRoot` test helper (8 test files migrated) Server services - `bulkUpdateCollectionItems` + `POST /api/media/collections/:id/items/bulk` with shared `validateItemInput` - `bulkReassignSeason` collapses N+1 writes in `deleteSeason` - `mediaAnnotations.readAll` hoists identity lookup, uses Promise.all - `annotationIdentity` lazy-subscribes to `settings:updated`, skips cache on settings-read failure (30s TTL invalidates on save) - Drop legacy `description` fallback in `sanitizeCharacter` + migration 017 - Universe sanitizer parity (already done in #255) agentPromptBuilder - Extract `buildTaskBlock`, `buildReviewLoopFollowUpSection`, `buildCompletionGuidelineBullet` - Tests for missing `worktreeCommitGuidance` branches Client - `` component (active+Legacy optgroup pattern, getLabel prop) - `CollectionPickerShell` shared by AddToCollectionMenu + BulkTargetPicker - `useLocalStorageBool` / `useLocalStoragePersisted` hooks (6 sites) - `apiCore.request()` honors FormData (skips JSON content-type) - `useMediaAnnotations.getCardProps(key)` (6 sites across 4 gallery pages) - `mergeVariations` drops malformed rows from both sides 5127 server tests pass, client build green. --- PLAN.md | 20 -- client/src/components/ModelSelect.jsx | 34 +++ .../components/meatspace/tabs/SettingsTab.jsx | 4 +- .../components/media/AddToCollectionMenu.jsx | 262 ++++-------------- .../src/components/media/BulkTargetPicker.jsx | 211 +++----------- .../media/CollectionPickerShell.jsx | 253 +++++++++++++++++ .../components/pipeline/stages/AudioStage.jsx | 4 +- client/src/hooks/useLocalStorageBool.js | 93 +++++++ client/src/hooks/useMediaAnnotations.js | 12 +- client/src/pages/ChiefOfStaff.jsx | 15 +- client/src/pages/CreativeDirector.jsx | 14 +- client/src/pages/ImageGen.jsx | 10 +- client/src/pages/Instances.jsx | 11 +- client/src/pages/MediaCollectionDetail.jsx | 15 +- client/src/pages/MediaHistory.jsx | 5 +- client/src/pages/PipelineSeries.jsx | 17 +- client/src/pages/UniverseBuilder.jsx | 47 ++-- client/src/pages/VideoGen.jsx | 24 +- client/src/pages/WritersRoom.jsx | 13 +- client/src/services/apiCore.js | 11 +- client/src/services/apiHealth.js | 20 +- client/src/services/apiPipeline.js | 25 +- .../017-character-description-to-physical.js | 118 ++++++++ server/lib/ffmpeg.js | 141 +++++++--- server/lib/ffmpeg.test.js | 68 ++++- server/lib/fileUtils.js | 49 +++- server/lib/fileUtils.test.js | 81 ++++++ server/lib/mockPathsDataRoot.js | 110 ++++++++ server/lib/mockPathsDataRoot.test.js | 87 ++++++ server/lib/storyBible.js | 56 ++-- server/lib/storyBible.test.js | 19 +- server/lib/validation.js | 30 ++ server/routes/mediaCollections.js | 14 +- server/routes/mediaCollections.test.js | 101 +++++++ server/services/agentPromptBuilder.js | 253 ++++++++++++----- server/services/agentPromptBuilder.test.js | 41 +++ server/services/imageGen/local.js | 35 +-- server/services/loras.js | 80 +++--- server/services/mediaAnnotations.js | 22 +- server/services/mediaCollections.js | 94 ++++++- server/services/mediaCollections.test.js | 83 ++++++ server/services/pipeline/audioMux.js | 35 +-- server/services/pipeline/audioMux.test.js | 13 +- server/services/pipeline/issues.js | 44 +++ server/services/pipeline/issues.test.js | 55 ++++ server/services/pipeline/musicLibrary.js | 40 +-- server/services/pipeline/seasons.js | 32 +-- server/services/pipeline/textStages.test.js | 2 +- server/services/settings.js | 12 + server/services/sharing/annotationIdentity.js | 50 +++- server/services/sharing/buckets.test.js | 8 +- server/services/sharing/manifest.test.js | 8 +- .../services/writersRoom/characters.test.js | 8 +- server/services/writersRoom/local.test.js | 10 +- server/services/writersRoom/objects.test.js | 8 +- .../writersRoom/promoteToPipeline.test.js | 8 +- server/services/writersRoom/settings.test.js | 8 +- 57 files changed, 2052 insertions(+), 891 deletions(-) create mode 100644 client/src/components/ModelSelect.jsx create mode 100644 client/src/components/media/CollectionPickerShell.jsx create mode 100644 client/src/hooks/useLocalStorageBool.js create mode 100644 scripts/migrations/017-character-description-to-physical.js create mode 100644 server/lib/mockPathsDataRoot.js create mode 100644 server/lib/mockPathsDataRoot.test.js create mode 100644 server/routes/mediaCollections.test.js diff --git a/PLAN.md b/PLAN.md index 138649574..b5c0ae000 100644 --- a/PLAN.md +++ b/PLAN.md @@ -99,31 +99,11 @@ For project goals, see [GOALS.md](./GOALS.md). For completed work, see [DONE.md] - [ ] **Extract `useSwipeNav` hook + `lib/clipboard.js`.** `MediaLightbox` swipe nav; clipboard inlined across 8+ call sites. Clipboard can move now. - [ ] **Route `MediaLightbox` settings drawer through `components/Drawer.jsx`.** Reconcile `Drawer`'s flat Esc handler with the lightbox's layered Escape cascade. -- [ ] **Extract `` component for the active+Legacy optgroup pattern.** `VideoGen.jsx` + `CreativeDirector.jsx` render identical blocks differing only in `m.name` vs `m.name || m.id`. -- [ ] **Extract `mockPathsDataRoot()` test helper.** 6 test files open with byte-identical PATHS mocking setup. - [ ] **`useAsyncAction` post-unmount setState guard.** Add `mountedRef` to gate `setRunning(false)`. YAGNI today; do at 4th consumer. -- [ ] **Extract `CollectionPickerShell` from `AddToCollectionMenu` + `BulkTargetPicker`.** Two near-identical portal popovers. While extracting, accept a `collections` prop so `MediaCollectionDetail` avoids per-mount `listMediaCollections` round-trip. -- [ ] **Server-side bulk endpoint for collection items.** `POST /api/media/collections/:id/items/bulk` taking `{ add, remove }` — single read-modify-write per collection. Halves wall-clock for Move; dodges N-call race window. Real use case: "select all" on world-builder collections with 50+ items. -- [ ] **Drop legacy `description` fallback in `sanitizeCharacter`.** Migrate `series.characters[].description` → `physicalDescription`; drop `|| raw.description` at `storyBible.js:246`. -- [ ] **Migrate `worldBuilderRefine.runRefine` onto `runPromptThroughProvider`.** Last LLM-runner site still hand-rolling the createRun → branch → executeApi/CliRun → accumulate-text pattern. ~30 LOC dedup. -- [ ] **Extract `usePersistedState` hook.** Six components repeat `useState(() => localStorage.getItem(KEY) === '1')` + setter. Add `useLocalStorageBool(key, default)` + JSON-blob variant. -- [ ] **Lift `runFfmpegProcess({ args, signal, stderrTailBytes }) → { ok, reason }` into `server/lib/ffmpeg.js`.** Three sites share spawn → stderr-tail → close → SIGTERM-on-abort. Leave `videoTimeline/local.js` (broadcast complicates it). - [ ] **Scene-level wardrobe picking.** Per-scene `characterAppearances: [{ characterId, wardrobeId? }]` on storyboard scenes with wardrobe-picker dropdown. Decide first: does the extractor guess or does the user pick? Append wardrobe after physicalDescription vs substitute body fields? -- [ ] **Extract `sanitizeListWith(raw, sanitizer, cap)` helper in `storyBible.js`.** Three array-walk + per-item sanitize + cap sites. - [ ] **Extract `useCanonPatch(universe, setUniverse, universeId, mountedRef)`.** `UniverseCanon.jsx` + `NounsStage.jsx` 95% identical optimistic-patch handlers. Extract when a 3rd caller appears. -- [ ] **AbortSignal listener cleanup in `audioMux.js#runFfmpeg`.** `addEventListener('abort', kill, { once: true })` never removed on normal completion. Theoretical until the stitch step passes a signal. -- [ ] **Extract `listDirectoryByExtension(dir, { extensions, mapEntry })`.** Three readdir + filter + stat-per-entry sites in `fileUtils.js`. -- [ ] **Teach `request()` in `apiCore.js` about FormData.** Drop hard-coded `Content-Type: application/json` when body is FormData. Two helpers (`apiHealth#uploadAppleHealthXml`, `apiPipeline#uploadPipelineMusicTrack`) bypass `request()` today. -- [ ] **`mergeVariations` NPE guard in `UniverseBuilder.jsx`.** Add `.label?.toLowerCase()` + `.filter(Boolean)` parity to the locked variations Set (post-rename file/line drift — agent confirmed line 57 still lacks the guard). - [ ] **Client tests for deep routing + drag.** Smoke tests for `goToWorld(id)` URL transitions and chip-reorder ordering (mock `useSortable`). -- [ ] **`useMediaAnnotations.getCardProps(key)` helper.** Single `{ starred, hasNote, onToggleStar }` lookup at 6 sites across 4 gallery pages. -- [ ] **Cache `resolveGlobalDisplayName()` settings read.** Memoize for ~30s or invalidate on `settings:updated` event. - [ ] **Shallow-equal guard in `useMediaAnnotations` socket handler.** Speculative micro-opt; theoretical until observed. -- [ ] **Hoist identity lookup out of `liftLegacyEntry` loop in `mediaAnnotations.js#readAll()`.** Pass `localInstanceId`, `defaultAuthorName` in once; make `liftLegacyEntry` synchronous. -- [ ] **Bulk reassign helper to collapse N+1 writes in `deleteSeason`.** Add `issuesSvc.bulkReassignSeason(seriesId, fromSeasonId, toSeasonId)` — one readState + N in-memory mutations + one writeState + one renumber. -- [ ] **Extract shared task block + review-loop section in `agentPromptBuilder.js`.** The light path (`buildLightContextPrompt` ~line 510) and full path (`buildAgentPrompt` fallback ~line 425) now emit nearly identical task blocks (`description` + optional `**Target App**:` + optional `**Screenshots**:`). The review-loop follow-up section is duplicated between the full-context block (~line 312-343) and the light-context block (~line 575-602) — same 7-step loop, same `gh pr merge --squash --delete-branch` command, same `gh pr view --json state` verification, same "10 iterations" hard stop. The auto-merge bug fix (PR removing `--auto`) had to be applied in both places. Extract `buildTaskBlock(task, { screenshotsAsList })` + `buildReviewLoopFollowUpSection(metadata, { verbose })` so the next sync edit doesn't drift. -- [ ] **Flatten the giant `isReadOnly ? … : isTui ? … : worktreeInfo && willOpenPR ? …` ternary in `agentPromptBuilder.js` Guidelines bullet (~line 458).** Pre-existing 4-branch one-liner that covers readOnly/TUI/worktree+PR/worktree-only/default-empty. Extract a `buildCompletionGuidelineBullet({ isReadOnly, isTui, tuiCompletionCommand, worktreeInfo, willOpenPR, willReviewLoop })` helper mirroring the pattern the light path already uses (`worktreeCommitGuidance`, `buildTuiCompletionSection`). Returns string or `null`. -- [ ] **Test the untested `worktreeCommitGuidance` branches in `agentPromptBuilder.js`.** Five-branch decision tree at ~line 617; tests don't cover `isWorktreeOnExistingBranch=true` or `hasSlashdo && !willOpenPR`. - [ ] **Extract a `tryReadFile(path, encoding='utf8')` helper into `server/lib/fileUtils.js`.** The `readFile(path).catch(() => null)` pattern is inlined 15+ times across the codebase (`server/routes/apps.js:38` `safeReadJson`, `server/lib/fileUtils.js:505`, `server/lib/hfToken.js:16`, `server/services/agentDrafts.js:22`, `server/services/messageAccounts.js:10`, `server/services/missions.js:35`, `server/lib/tuiPromptRunner.js` new at line ~202, etc.). One helper + migrate; small win, prevents future drift. Defer until next infra pass. - [ ] **Skip the `text` accumulator in `promptRunner.runPromptThroughProvider` for TUI providers.** `APPEND_CHUNK(acc, chunk)` runs per stream chunk regardless of provider type, but the TUI branch (post `fix-prose-stage-override`) discards the accumulated `text` and uses `result.text` from `executeTuiRun` instead. Streams can hit hundreds of KB of screen redraws per run — gate the accumulator on `effectiveProvider.type !== 'tui'` in `onData` to skip the per-chunk string concat. Pre-existing buffer cap on `outputBuffer` inside `executeTuiRun` already bounds memory; this is a CPU/alloc micro-opt, not a correctness fix. diff --git a/client/src/components/ModelSelect.jsx b/client/src/components/ModelSelect.jsx new file mode 100644 index 000000000..28b7f3003 --- /dev/null +++ b/client/src/components/ModelSelect.jsx @@ -0,0 +1,34 @@ +// Active + Legacy optgroup ` + {active.map((m) => )} + {legacy.length > 0 && ( + + {legacy.map((m) => )} + + )} + + ); +} diff --git a/client/src/components/meatspace/tabs/SettingsTab.jsx b/client/src/components/meatspace/tabs/SettingsTab.jsx index f4bf7c6af..b5db4f1b4 100644 --- a/client/src/components/meatspace/tabs/SettingsTab.jsx +++ b/client/src/components/meatspace/tabs/SettingsTab.jsx @@ -93,7 +93,9 @@ export default function SettingsTab({ onRefresh }) { setXmlProgress(0); setXmlImporting(true); - api.uploadAppleHealthXml(file).catch(err => { + // Owns its own error UI (xmlError) so suppress the helper's toast — per + // CLAUDE.md "custom catch ⇒ silent: true" convention. + api.uploadAppleHealthXml(file, { silent: true }).catch(err => { setXmlError(err.message); setXmlImporting(false); }); diff --git a/client/src/components/media/AddToCollectionMenu.jsx b/client/src/components/media/AddToCollectionMenu.jsx index 34ed47294..6b7ccfe8b 100644 --- a/client/src/components/media/AddToCollectionMenu.jsx +++ b/client/src/components/media/AddToCollectionMenu.jsx @@ -1,24 +1,17 @@ -import { useEffect, useLayoutEffect, useRef, useState, useCallback, useMemo } from 'react'; -import { createPortal } from 'react-dom'; -import { FolderPlus, Check, Plus, Search } from 'lucide-react'; +import { useCallback, useRef, useState } from 'react'; +import { FolderPlus, Check } from 'lucide-react'; import toast from '../ui/Toast'; -import { - listMediaCollections, createMediaCollection, - addMediaCollectionItem, removeMediaCollectionItem, -} from '../../services/api'; +import CollectionPickerShell from './CollectionPickerShell'; +import { addMediaCollectionItem, removeMediaCollectionItem } from '../../services/api'; // Self-contained popover button used by MediaCard. The popover is portalled -// into with fixed positioning so it escapes the parent grid's -// `overflow-auto` clip and stacks above the sidebar (which is z-50). +// into by CollectionPickerShell with fixed positioning so it escapes +// the parent grid's `overflow-auto` clip and stacks above the sidebar. // -// `itemKey` is ":" — the same format the server uses for -// coverKey and for the DELETE /items/:key route. - -const MENU_WIDTH = 240; -const MENU_GAP = 6; -const VIEWPORT_PADDING = 8; -// Show search affordance only when the list could meaningfully scroll. -const SEARCH_THRESHOLD = 6; +// `itemKey` is ":" — the same format the server uses for coverKey +// and for the DELETE /items/:key route. +// +// The shell is now shared with BulkTargetPicker (see CollectionPickerShell.jsx). // Only the trigger button varies by size — popover styling is fixed. const SIZES = { @@ -28,115 +21,18 @@ const SIZES = { export default function AddToCollectionMenu({ item, size = 'sm' }) { const [open, setOpen] = useState(false); - const [collections, setCollections] = useState(null); // null = not loaded - const [creating, setCreating] = useState(false); - const [newName, setNewName] = useState(''); const [busyId, setBusyId] = useState(null); - const [query, setQuery] = useState(''); - const [menuStyle, setMenuStyle] = useState(null); const triggerRef = useRef(null); - const menuRef = useRef(null); const itemKey = item.key; const itemRef = item.kind === 'video' ? item.id : item.filename; - const filtered = useMemo(() => { - if (!collections) return null; - const q = query.trim().toLowerCase(); - if (!q) return collections; - return collections.filter((c) => c.name.toLowerCase().includes(q)); - }, [collections, query]); - - const updateMenuPosition = useCallback(() => { - const trigger = triggerRef.current; - const menu = menuRef.current; - if (!trigger || !menu) return; - - const viewportWidth = window.innerWidth; - const viewportHeight = window.innerHeight; - const width = Math.min(MENU_WIDTH, Math.max(180, viewportWidth - VIEWPORT_PADDING * 2)); - menu.style.width = `${width}px`; - - const triggerRect = trigger.getBoundingClientRect(); - const menuRect = menu.getBoundingClientRect(); - - const maxLeft = viewportWidth - width - VIEWPORT_PADDING; - const left = Math.min( - Math.max(triggerRect.right - width, VIEWPORT_PADDING), - Math.max(VIEWPORT_PADDING, maxLeft), - ); - - const aboveTop = triggerRect.top - menuRect.height - MENU_GAP; - const belowTop = triggerRect.bottom + MENU_GAP; - const wouldOverflowTop = aboveTop < VIEWPORT_PADDING; - let top = wouldOverflowTop ? belowTop : aboveTop; - const maxTop = Math.max(VIEWPORT_PADDING, viewportHeight - menuRect.height - VIEWPORT_PADDING); - top = Math.min(Math.max(top, VIEWPORT_PADDING), maxTop); - - setMenuStyle((prev) => { - const next = { left: `${left}px`, top: `${top}px`, width: `${width}px` }; - if (prev && prev.left === next.left && prev.top === next.top && prev.width === next.width) return prev; - return next; - }); - }, []); - - useEffect(() => { - if (!open) return undefined; - const onClickAway = (e) => { - const onTrigger = triggerRef.current?.contains(e.target); - const onMenu = menuRef.current?.contains(e.target); - if (!onTrigger && !onMenu) setOpen(false); - }; - const onEsc = (e) => { if (e.key === 'Escape') setOpen(false); }; - let rafId = null; - const onReposition = () => { - if (rafId !== null) return; - rafId = requestAnimationFrame(() => { - rafId = null; - updateMenuPosition(); - }); - }; - document.addEventListener('mousedown', onClickAway); - document.addEventListener('keydown', onEsc); - window.addEventListener('resize', onReposition); - window.addEventListener('scroll', onReposition, true); - return () => { - if (rafId !== null) cancelAnimationFrame(rafId); - document.removeEventListener('mousedown', onClickAway); - document.removeEventListener('keydown', onEsc); - window.removeEventListener('resize', onReposition); - window.removeEventListener('scroll', onReposition, true); - }; - }, [open, updateMenuPosition]); - - useLayoutEffect(() => { - if (!open) { - setMenuStyle(null); - return; - } - updateMenuPosition(); - }, [open, updateMenuPosition, collections, filtered, query]); - - const ensureLoaded = async () => { - if (collections != null) return; - const data = await listMediaCollections().catch((err) => { - toast.error(err.message || 'Failed to load collections'); - return []; - }); - setCollections(Array.isArray(data) ? data : []); - }; - - const handleToggleOpen = async (e) => { + const handleToggleOpen = (e) => { e.stopPropagation(); - const next = !open; - setOpen(next); - if (next) { - setQuery(''); - await ensureLoaded(); - } + setOpen((prev) => !prev); }; - const handleToggleMembership = async (collection) => { + const handleToggleMembership = useCallback(async (collection, updateCollections) => { const inIt = collection.items.some((it) => `${it.kind}:${it.ref}` === itemKey); setBusyId(collection.id); const updated = inIt @@ -150,37 +46,46 @@ export default function AddToCollectionMenu({ item, size = 'sm' }) { }); setBusyId(null); if (!updated) return; - setCollections((prev) => (prev || []).map((c) => (c.id === collection.id ? updated : c))); + updateCollections((prev) => (prev || []).map((c) => (c.id === collection.id ? updated : c))); toast.success(inIt ? `Removed from ${collection.name}` : `Added to ${collection.name}`); + }, [itemKey, itemRef, item.kind]); + + const renderItem = (c, { updateCollections }) => { + const inIt = c.items.some((it) => `${it.kind}:${it.ref}` === itemKey); + return ( + + ); }; - const handleCreate = async (e) => { - e.preventDefault(); - const name = newName.trim(); - if (!name) return; - setCreating(true); - const created = await createMediaCollection({ name }).catch((err) => { - toast.error(err.message || 'Create failed'); - return null; - }); - if (!created) { setCreating(false); return; } + const handleCreated = useCallback(async (created, { addToCollectionsState }) => { + // For this variant of the picker, creating a collection means "add the + // current item to the new collection." Surface the underlying-add failure + // separately from the create itself so the user knows the collection + // exists even if the auto-add failed. let addError = null; const withItem = await addMediaCollectionItem(created.id, { kind: item.kind, ref: itemRef }).catch((err) => { addError = err; return created; }); - setCollections((prev) => ([...(prev || []), withItem])); - setNewName(''); - setCreating(false); + addToCollectionsState((prev) => ([...(prev || []), withItem])); if (addError) { toast.error(`Created "${created.name}" but failed to add: ${addError.message}`); } else { toast.success(`Added to ${created.name}`); } - }; + }, [item.kind, itemRef]); - const showSearch = (collections?.length || 0) >= SEARCH_THRESHOLD; - const list = filtered ?? collections; const sizeCls = SIZES[size] || SIZES.sm; return ( @@ -196,83 +101,18 @@ export default function AddToCollectionMenu({ item, size = 'sm' }) { > - {open && createPortal( -
e.stopPropagation()} - role="menu" - > -
Collections
- {showSearch && ( -
- - setQuery(e.target.value)} - placeholder="Search collections…" - className="w-full bg-port-bg border border-port-border rounded pl-7 pr-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent" - autoFocus - /> -
- )} -
- {collections == null && ( -
Loading…
- )} - {collections != null && collections.length === 0 && ( -
No collections yet — create one below.
- )} - {list != null && collections?.length > 0 && list.length === 0 && ( -
No matches for “{query}”
- )} - {list != null && list.map((c) => { - const inIt = c.items.some((it) => `${it.kind}:${it.ref}` === itemKey); - return ( - - ); - })} -
-
- setNewName(e.target.value)} - placeholder="New collection name" - maxLength={80} - className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent" - /> - -
-
, - document.body, - )} + setOpen(false)} + title="Collections" + width={240} + minWidth={180} + newCollectionPlaceholder="New collection name" + createTitle="Create and add" + renderItem={renderItem} + onCreated={handleCreated} + /> ); } diff --git a/client/src/components/media/BulkTargetPicker.jsx b/client/src/components/media/BulkTargetPicker.jsx index a9974896c..7694c664b 100644 --- a/client/src/components/media/BulkTargetPicker.jsx +++ b/client/src/components/media/BulkTargetPicker.jsx @@ -1,185 +1,62 @@ -import { useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; -import { createPortal } from 'react-dom'; -import { Plus, Search } from 'lucide-react'; -import toast from '../ui/Toast'; -import { listMediaCollections, createMediaCollection } from '../../services/api'; +import { useCallback } from 'react'; +import CollectionPickerShell from './CollectionPickerShell'; // Single-target picker used by MediaCollectionDetail's bulk-action bar. // AddToCollectionMenu toggles membership for one item across many collections; // this picker is the inverse — one click picks a destination for many items. // -// Sibling-component prior art (AddToCollectionMenu) handles popover positioning -// the same way: portal into body, fixed coords, recompute on scroll/resize. - -const MENU_WIDTH = 260; -const MENU_GAP = 6; -const VIEWPORT_PADDING = 8; -const SEARCH_THRESHOLD = 6; +// Both share the same popover/positioning/create-form shell — see +// CollectionPickerShell.jsx. The differences this caller layers on: +// - rows render with an item-count badge instead of a membership checkmark +// - clicking a row calls onPick(id, name) instead of toggling membership +// - creating a new collection picks it as the destination +// +// `collections` (optional): the parent's already-fetched list, so the shell +// skips its own listMediaCollections() round-trip. MediaCollectionDetail +// passes this to dedupe the load it already performed. export default function BulkTargetPicker({ anchorRef, excludeId, busy, title = 'Pick a collection', + collections, onPick, onClose, }) { - const [collections, setCollections] = useState(null); - const [query, setQuery] = useState(''); - const [creating, setCreating] = useState(false); - const [newName, setNewName] = useState(''); - const [style, setStyle] = useState(null); - const menuRef = useRef(null); - // Parent passes inline arrows for onClose; reading it through a ref keeps - // the listener effect from tearing down + re-attaching every parent render. - const onCloseRef = useRef(onClose); - useEffect(() => { onCloseRef.current = onClose; }); - - const filtered = useMemo(() => { - if (!collections) return null; - const base = excludeId ? collections.filter((c) => c.id !== excludeId) : collections; - const q = query.trim().toLowerCase(); - return q ? base.filter((c) => c.name.toLowerCase().includes(q)) : base; - }, [collections, query, excludeId]); - - useEffect(() => { - listMediaCollections().then( - (data) => setCollections(Array.isArray(data) ? data : []), - (err) => { toast.error(err?.message || 'Failed to load collections'); setCollections([]); }, - ); - }, []); - - const reposition = () => { - const trigger = anchorRef?.current; - const menu = menuRef.current; - if (!trigger || !menu) return; - const vw = window.innerWidth; - const vh = window.innerHeight; - const width = Math.min(MENU_WIDTH, Math.max(200, vw - VIEWPORT_PADDING * 2)); - menu.style.width = `${width}px`; - const tr = trigger.getBoundingClientRect(); - const mr = menu.getBoundingClientRect(); - const maxLeft = vw - width - VIEWPORT_PADDING; - const left = Math.min(Math.max(tr.right - width, VIEWPORT_PADDING), Math.max(VIEWPORT_PADDING, maxLeft)); - const above = tr.top - mr.height - MENU_GAP; - const below = tr.bottom + MENU_GAP; - const top = above < VIEWPORT_PADDING ? below : above; - const maxTop = Math.max(VIEWPORT_PADDING, vh - mr.height - VIEWPORT_PADDING); - const clampedTop = Math.min(Math.max(top, VIEWPORT_PADDING), maxTop); - setStyle({ left: `${left}px`, top: `${clampedTop}px`, width: `${width}px` }); - }; - - useLayoutEffect(reposition, [filtered, excludeId]); - useEffect(() => { - const close = () => onCloseRef.current?.(); - const onKey = (e) => { if (e.key === 'Escape') close(); }; - const onAway = (e) => { - const onTrigger = anchorRef?.current?.contains(e.target); - const onMenu = menuRef.current?.contains(e.target); - if (!onTrigger && !onMenu) close(); - }; - let raf = null; - const onScroll = () => { if (raf !== null) return; raf = requestAnimationFrame(() => { raf = null; reposition(); }); }; - document.addEventListener('keydown', onKey); - document.addEventListener('mousedown', onAway); - window.addEventListener('resize', onScroll); - window.addEventListener('scroll', onScroll, true); - return () => { - if (raf !== null) cancelAnimationFrame(raf); - document.removeEventListener('keydown', onKey); - document.removeEventListener('mousedown', onAway); - window.removeEventListener('resize', onScroll); - window.removeEventListener('scroll', onScroll, true); - }; - }, []); + const renderItem = useCallback((c) => ( + + ), [busy, onPick]); - const handleCreate = async (e) => { - e.preventDefault(); - const name = newName.trim(); - if (!name) return; - setCreating(true); - const created = await createMediaCollection({ name }).catch((err) => { - toast.error(err?.message || 'Create failed'); - return null; - }); - setCreating(false); - if (!created) return; - setNewName(''); + const handleCreated = useCallback((created) => { onPick(created.id, created.name); - }; - - const showSearch = (collections?.length || 0) >= SEARCH_THRESHOLD; - const list = filtered ?? collections; - - return createPortal( -
e.stopPropagation()} - > -
{title}
- {showSearch && ( -
- - setQuery(e.target.value)} - placeholder="Search collections…" - className="w-full bg-port-bg border border-port-border rounded pl-7 pr-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent" - autoFocus - /> -
- )} -
- {collections == null &&
Loading…
} - {collections != null && list?.length === 0 && ( -
- {query ? `No matches for "${query}"` : 'No other collections — create one below.'} -
- )} - {list?.map((c) => ( - - ))} -
-
- setNewName(e.target.value)} - placeholder="New collection" - maxLength={80} - disabled={busy} - className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent disabled:opacity-50" - /> - -
-
, - document.body, + }, [onPick]); + + return ( + (q ? `No matches for "${q}"` : 'No other collections — create one below.')} + newCollectionPlaceholder="New collection" + createTitle="Create and pick" + collections={collections} + renderItem={renderItem} + onCreated={handleCreated} + /> ); } diff --git a/client/src/components/media/CollectionPickerShell.jsx b/client/src/components/media/CollectionPickerShell.jsx new file mode 100644 index 000000000..d131cb561 --- /dev/null +++ b/client/src/components/media/CollectionPickerShell.jsx @@ -0,0 +1,253 @@ +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; +import { createPortal } from 'react-dom'; +import { Plus, Search } from 'lucide-react'; +import toast from '../ui/Toast'; +import { listMediaCollections, createMediaCollection } from '../../services/api'; + +// Shared popover shell for the two collection pickers: +// +// - AddToCollectionMenu (one item → many collections, toggles membership) +// - BulkTargetPicker (many items → one destination, pick + dispatch) +// +// Both portal a fixed-position popover into , compute placement relative +// to a trigger element, ship a search input above a threshold, and end with a +// "create new" inline form. This shell owns all of that plumbing; the caller +// provides `renderItem` for the per-row UI and `onCreated`/`onPickCreated` for +// what to do after the inline create succeeds. +// +// Collections data flow (per PLAN.md item 2): +// - If `collections` is provided by the parent, we never call +// `listMediaCollections()` ourselves — this lets MediaCollectionDetail +// reuse its already-fetched list instead of paying a per-mount round-trip. +// - If `collections` is `null`/omitted, the shell auto-loads on mount (old +// AddToCollectionMenu behavior). Use `onCollectionsLoaded` if the parent +// wants to learn the fetched list (e.g. for membership rendering). +// +// `excludeId` is a single-item allow-list filter so BulkTargetPicker can hide +// the current collection from its move/copy target list. + +const DEFAULT_MENU_WIDTH = 260; +const MENU_GAP = 6; +const VIEWPORT_PADDING = 8; +const SEARCH_THRESHOLD = 6; + +export default function CollectionPickerShell({ + anchorRef, + open = true, + title, + emptyMessage = 'No collections yet — create one below.', + noMatchMessage = (query) => `No matches for "${query}"`, + excludeId, + busy = false, + width = DEFAULT_MENU_WIDTH, + minWidth = 200, + newCollectionPlaceholder = 'New collection', + createTitle = 'Create and pick', + // The caller renders each collection row — keeps the per-row UI flexible + // (membership checkmarks vs item counts vs anything else) without dragging + // every prop combination through the shell. + renderItem, + collections: collectionsProp, + onCollectionsLoaded, + onCollectionsChange, + onCreated, + onClose, +}) { + const [collectionsState, setCollectionsState] = useState(collectionsProp ?? null); + const [query, setQuery] = useState(''); + const [creating, setCreating] = useState(false); + const [newName, setNewName] = useState(''); + const [style, setStyle] = useState(null); + const menuRef = useRef(null); + + // Parents may pass inline arrow handlers — read through a ref so the + // event-listener effect doesn't tear down on every parent render. + const onCloseRef = useRef(onClose); + useEffect(() => { onCloseRef.current = onClose; }); + + // When the parent owns `collections`, mirror it into local state on each + // change so the same render path works for both modes. + useEffect(() => { + if (collectionsProp !== undefined) setCollectionsState(collectionsProp); + }, [collectionsProp]); + + // Auto-load only when uncontrolled (no `collections` prop). Mirrors the old + // AddToCollectionMenu behavior of fetching when the popover opens. + useEffect(() => { + if (collectionsProp !== undefined) return undefined; + if (!open) return undefined; + let cancelled = false; + listMediaCollections().then( + (data) => { + if (cancelled) return; + const list = Array.isArray(data) ? data : []; + setCollectionsState(list); + onCollectionsLoaded?.(list); + }, + (err) => { + if (cancelled) return; + toast.error(err?.message || 'Failed to load collections'); + setCollectionsState([]); + }, + ); + return () => { cancelled = true; }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open, collectionsProp]); + + const filtered = useMemo(() => { + if (!collectionsState) return null; + const base = excludeId ? collectionsState.filter((c) => c.id !== excludeId) : collectionsState; + const q = query.trim().toLowerCase(); + return q ? base.filter((c) => c.name.toLowerCase().includes(q)) : base; + }, [collectionsState, query, excludeId]); + + const reposition = useCallback(() => { + const trigger = anchorRef?.current; + const menu = menuRef.current; + if (!trigger || !menu) return; + const vw = window.innerWidth; + const vh = window.innerHeight; + const w = Math.min(width, Math.max(minWidth, vw - VIEWPORT_PADDING * 2)); + menu.style.width = `${w}px`; + const tr = trigger.getBoundingClientRect(); + const mr = menu.getBoundingClientRect(); + const maxLeft = vw - w - VIEWPORT_PADDING; + const left = Math.min(Math.max(tr.right - w, VIEWPORT_PADDING), Math.max(VIEWPORT_PADDING, maxLeft)); + const above = tr.top - mr.height - MENU_GAP; + const below = tr.bottom + MENU_GAP; + const top = above < VIEWPORT_PADDING ? below : above; + const maxTop = Math.max(VIEWPORT_PADDING, vh - mr.height - VIEWPORT_PADDING); + const clampedTop = Math.min(Math.max(top, VIEWPORT_PADDING), maxTop); + setStyle((prev) => { + const next = { left: `${left}px`, top: `${clampedTop}px`, width: `${w}px` }; + if (prev && prev.left === next.left && prev.top === next.top && prev.width === next.width) return prev; + return next; + }); + }, [anchorRef, width, minWidth]); + + useLayoutEffect(() => { + if (!open) { setStyle(null); return; } + reposition(); + }, [open, reposition, filtered, query, collectionsState]); + + useEffect(() => { + if (!open) return undefined; + const close = () => onCloseRef.current?.(); + const onKey = (e) => { if (e.key === 'Escape') close(); }; + const onAway = (e) => { + const onTrigger = anchorRef?.current?.contains(e.target); + const onMenu = menuRef.current?.contains(e.target); + if (!onTrigger && !onMenu) close(); + }; + let raf = null; + const onScroll = () => { if (raf !== null) return; raf = requestAnimationFrame(() => { raf = null; reposition(); }); }; + document.addEventListener('keydown', onKey); + document.addEventListener('mousedown', onAway); + window.addEventListener('resize', onScroll); + window.addEventListener('scroll', onScroll, true); + return () => { + if (raf !== null) cancelAnimationFrame(raf); + document.removeEventListener('keydown', onKey); + document.removeEventListener('mousedown', onAway); + window.removeEventListener('resize', onScroll); + window.removeEventListener('scroll', onScroll, true); + }; + }, [open, anchorRef, reposition]); + + const updateCollections = useCallback((updater) => { + setCollectionsState((prev) => { + const next = typeof updater === 'function' ? updater(prev) : updater; + onCollectionsChange?.(next); + return next; + }); + }, [onCollectionsChange]); + + const handleCreate = async (e) => { + e.preventDefault(); + const name = newName.trim(); + if (!name) return; + setCreating(true); + const created = await createMediaCollection({ name }).catch((err) => { + toast.error(err?.message || 'Create failed'); + return null; + }); + setCreating(false); + if (!created) return; + setNewName(''); + // Parent decides what happens next — AddToCollectionMenu wants to also add + // the item to the new collection, BulkTargetPicker just picks it as the + // destination. The shell tells the parent which collection was created + // and how the parent should merge it back into the local list. + onCreated?.(created, { addToCollectionsState: (mergeFn) => updateCollections(mergeFn) }); + }; + + if (!open) return null; + + const showSearch = (collectionsState?.length || 0) >= SEARCH_THRESHOLD; + const list = filtered ?? collectionsState; + + return createPortal( +
e.stopPropagation()} + > + {title && ( +
{title}
+ )} + {showSearch && ( +
+ + setQuery(e.target.value)} + placeholder="Search collections…" + className="w-full bg-port-bg border border-port-border rounded pl-7 pr-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent" + autoFocus + /> +
+ )} +
+ {collectionsState == null && ( +
Loading…
+ )} + {collectionsState != null && collectionsState.length === 0 && ( +
{emptyMessage}
+ )} + {list != null && collectionsState?.length > 0 && list.length === 0 && ( +
{noMatchMessage(query)}
+ )} + {list != null && list.map((c) => renderItem(c, { updateCollections }))} +
+
+ setNewName(e.target.value)} + placeholder={newCollectionPlaceholder} + maxLength={80} + disabled={busy} + className="flex-1 bg-port-bg border border-port-border rounded px-2 py-1 text-[11px] text-white focus:outline-none focus:border-port-accent disabled:opacity-50" + /> + +
+
, + document.body, + ); +} diff --git a/client/src/components/pipeline/stages/AudioStage.jsx b/client/src/components/pipeline/stages/AudioStage.jsx index 0f8ec8989..6b56ca0c8 100644 --- a/client/src/components/pipeline/stages/AudioStage.jsx +++ b/client/src/components/pipeline/stages/AudioStage.jsx @@ -83,7 +83,9 @@ export default function AudioStage({ issue, onStageUpdate }) { const handleMusicUpload = async (file) => { if (!file) return; setMusicUploading(true); - const result = await uploadPipelineMusicTrack(issue.id, file).catch((err) => { + // Owns its own error UI (toast.error below) so suppress the helper's toast + // — per CLAUDE.md "custom catch ⇒ silent: true" convention. + const result = await uploadPipelineMusicTrack(issue.id, file, {}, { silent: true }).catch((err) => { toast.error(err.message || 'Upload failed'); return null; }); diff --git a/client/src/hooks/useLocalStorageBool.js b/client/src/hooks/useLocalStorageBool.js new file mode 100644 index 000000000..10c556028 --- /dev/null +++ b/client/src/hooks/useLocalStorageBool.js @@ -0,0 +1,93 @@ +import { useCallback, useState } from 'react'; + +// Boolean-valued `useState` mirror that round-trips through `localStorage`. +// +// History: six pages independently re-implemented this with subtly different +// encodings — `'1'/'0'`, `'true'/'false'`, sometimes wrapped in `try/catch` for +// sandboxed-storage failures, sometimes not. We accept both legacy encodings +// on read so swapping any one call site to the hook is non-breaking; writes +// are pinned to whichever format the page already uses (`format` option). +// +// `format`: `'1'` writes `'1'`/`'0'`; `'true'` writes `'true'`/`'false'`. +// Reads always treat either `'1'` or `'true'` as truthy so existing values +// keep working when a page switches its `format` later. +export function useLocalStorageBool(key, defaultValue = false, { format = '1' } = {}) { + const [value, setValue] = useState(() => readBool(key, defaultValue)); + + const write = useCallback((next) => { + setValue((prev) => { + const resolved = typeof next === 'function' ? next(prev) : next; + const bool = !!resolved; + writeBool(key, bool, format); + return bool; + }); + }, [key, format]); + + return [value, write]; +} + +// JSON-blob variant: hydrates from localStorage on first render, persists on +// every change via the setter. `defaultValue` is returned both when the key is +// missing and when JSON.parse fails (corrupted/old-shape value). +// +// Pass `{ parse: (raw) => merged }` to migrate older shapes — `parse` is only +// invoked when JSON.parse succeeded, so it sees the raw stored object. Use it +// to spread fresh defaults under saved values (additive shape changes) so a +// new field appears for users with pre-existing storage. +export function useLocalStoragePersisted(key, defaultValue, { parse } = {}) { + const [value, setValue] = useState(() => { + const raw = readJsonRaw(key); + if (raw === undefined) return defaultValue; + return parse ? parse(raw) : raw; + }); + + const write = useCallback((next) => { + setValue((prev) => { + const resolved = typeof next === 'function' ? next(prev) : next; + writeJson(key, resolved); + return resolved; + }); + }, [key]); + + return [value, write]; +} + +function readBool(key, defaultValue) { + if (typeof window === 'undefined') return defaultValue; + try { + const raw = window.localStorage.getItem(key); + if (raw === null) return defaultValue; + return raw === '1' || raw === 'true'; + } catch { + return defaultValue; + } +} + +function writeBool(key, bool, format) { + if (typeof window === 'undefined') return; + try { + const v = format === 'true' ? String(bool) : (bool ? '1' : '0'); + window.localStorage.setItem(key, v); + } catch { /* sandboxed storage */ } +} + +// Returns `undefined` to distinguish "key missing / parse failed" (caller +// applies the default) from a stored `null`. useLocalStoragePersisted uses +// this so it can run an optional `parse` migration only on real stored data. +function readJsonRaw(key) { + if (typeof window === 'undefined') return undefined; + try { + const raw = window.localStorage.getItem(key); + if (raw === null) return undefined; + return JSON.parse(raw); + } catch { + return undefined; + } +} + +function writeJson(key, value) { + if (typeof window === 'undefined') return; + try { + window.localStorage.setItem(key, JSON.stringify(value)); + } catch { /* sandboxed storage */ } +} diff --git a/client/src/hooks/useMediaAnnotations.js b/client/src/hooks/useMediaAnnotations.js index 594565eff..b8db53ef6 100644 --- a/client/src/hooks/useMediaAnnotations.js +++ b/client/src/hooks/useMediaAnnotations.js @@ -118,5 +118,15 @@ export function useMediaAnnotations() { updateAnnotation(item.key, { starred: !priorStarred }); }, [updateAnnotation]); - return { annotations, toggleStar, updateAnnotation }; + // Shortcut for the `{ starred, hasNote, onToggleStar }` triple that every + // consumer reads from this hook. Callers spread it onto the card + // and can still override individual fields (e.g. clear onToggleStar in + // select/stitch modes by passing `onToggleStar={undefined}` after the spread). + const getCardProps = useCallback((key) => ({ + starred: !!annotations[key]?.starred, + hasNote: !!annotations[key]?.anyNote, + onToggleStar: toggleStar, + }), [annotations, toggleStar]); + + return { annotations, toggleStar, updateAnnotation, getCardProps }; } diff --git a/client/src/pages/ChiefOfStaff.jsx b/client/src/pages/ChiefOfStaff.jsx index 9fabe500c..69c029cbb 100644 --- a/client/src/pages/ChiefOfStaff.jsx +++ b/client/src/pages/ChiefOfStaff.jsx @@ -1,6 +1,7 @@ import { useState, useEffect, useCallback, useMemo, useRef } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; import { useSocket } from '../hooks/useSocket'; +import { useLocalStorageBool } from '../hooks/useLocalStorageBool'; import * as api from '../services/api'; import { Play, Square, Clock, CheckCircle, AlertCircle, Cpu, ChevronDown, ChevronUp, ChevronLeft, ChevronRight, Brain, PanelLeftClose, PanelLeftOpen } from 'lucide-react'; import toast from '../components/ui/Toast'; @@ -62,8 +63,10 @@ export default function ChiefOfStaff() { const [liveOutputs, setLiveOutputs] = useState({}); const [eventLogs, setEventLogs] = useState([]); const [agentPanelCollapsed, setAgentPanelCollapsed] = useState(false); - const [desktopPanelCollapsed, setDesktopPanelCollapsed] = useState(() => - localStorage.getItem('cos-panel-collapsed') === 'true' + const [desktopPanelCollapsed, setDesktopPanelCollapsed] = useLocalStorageBool( + 'cos-panel-collapsed', + false, + { format: 'true' }, ); const [activeAgentMeta, setActiveAgentMeta] = useState(null); const [learningSummary, setLearningSummary] = useState(null); @@ -85,12 +88,8 @@ export default function ChiefOfStaff() { }; const toggleDesktopPanel = useCallback(() => { - setDesktopPanelCollapsed(prev => { - const next = !prev; - localStorage.setItem('cos-panel-collapsed', String(next)); - return next; - }); - }, []); + setDesktopPanelCollapsed((prev) => !prev); + }, [setDesktopPanelCollapsed]); // Countdown to next evaluation const evalCountdown = useNextEvalCountdown( diff --git a/client/src/pages/CreativeDirector.jsx b/client/src/pages/CreativeDirector.jsx index 1acc2dc41..5e83b7b53 100644 --- a/client/src/pages/CreativeDirector.jsx +++ b/client/src/pages/CreativeDirector.jsx @@ -11,6 +11,7 @@ import { pauseCreativeDirectorProject, } from '../services/apiCreativeDirector.js'; import { listVideoModels } from '../services/apiImageVideo.js'; +import ModelSelect from '../components/ModelSelect'; const ASPECT_RATIOS = ['16:9', '9:16', '1:1']; const QUALITIES = ['draft', 'standard', 'high']; @@ -184,18 +185,13 @@ export default function CreativeDirector() {