From 1aafcb49726c36f20d0f937bf49bd9ccae0e183d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20F=C3=B6rtsch?= Date: Mon, 13 Apr 2026 12:01:57 +0200 Subject: [PATCH] apply codex code review: fix useEffect refetch loops, dead routes, subtitle job_type leftovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All ack'd as real bugs: frontend - AudioDetailPage / SubtitleDetailPage / PathsPage / ScanPage / SubtitleListPage / ExecutePage: load() was a fresh function reference every render, so 'useEffect(() => load(), [load])' refetched on every render. Wrap each in useCallback with the right deps ([id], [filter], or []). - SetupPage: langsLoaded was useState; setting it inside load() retriggered the same effect → infinite loop. Switch to useRef. Also wrap saveJellyfin/ Radarr/Sonarr in async fns so they return Promise (matches the consumer signatures, fixes the latent TS error). - DashboardPage: redirect target /setup doesn't exist; the route is /settings. - ExecutePage: <>... fragment with two children had keys on the rows but not on the fragment → React reconciliation warning. Use . jobTypeLabel + badge variant still branched on the removed 'subtitle' job_type — relabel to 'Audio Transcode' / 'Audio Remux' and use 'manual'/'noop' variants. server - review.ts + scan.ts: parseLanguageList helper catches JSON errors and enforces array-of-strings shape with a fallback. A corrupted config row would otherwise throw mid-scan. --- server/api/review.ts | 20 ++++++- server/api/scan.ts | 14 ++++- src/features/dashboard/DashboardPage.tsx | 2 +- src/features/execute/ExecutePage.tsx | 57 ++++++++++--------- src/features/paths/PathsPage.tsx | 6 +- src/features/review/AudioDetailPage.tsx | 21 ++++--- src/features/scan/ScanPage.tsx | 4 +- src/features/setup/SetupPage.tsx | 24 +++++--- src/features/subtitles/SubtitleDetailPage.tsx | 21 ++++--- src/features/subtitles/SubtitleListPage.tsx | 6 +- 10 files changed, 107 insertions(+), 68 deletions(-) diff --git a/server/api/review.ts b/server/api/review.ts index 7609400..9e07eb6 100644 --- a/server/api/review.ts +++ b/server/api/review.ts @@ -11,7 +11,21 @@ const app = new Hono(); // ─── Helpers ────────────────────────────────────────────────────────────────── function getSubtitleLanguages(): string[] { - return JSON.parse(getConfig("subtitle_languages") ?? '["eng","deu","spa"]'); + return parseLanguageList(getConfig("subtitle_languages"), ["eng", "deu", "spa"]); +} + +function getAudioLanguages(): string[] { + return parseLanguageList(getConfig("audio_languages"), []); +} + +function parseLanguageList(raw: string | null, fallback: string[]): string[] { + if (!raw) return fallback; + try { + const parsed = JSON.parse(raw); + return Array.isArray(parsed) ? parsed.filter((v): v is string => typeof v === "string") : fallback; + } catch { + return fallback; + } } function countsByFilter(db: ReturnType): Record { @@ -114,7 +128,7 @@ function reanalyze(db: ReturnType, itemId: number, preservedTitles .prepare("SELECT * FROM media_streams WHERE item_id = ? ORDER BY stream_index") .all(itemId) as MediaStream[]; const subtitleLanguages = getSubtitleLanguages(); - const audioLanguages: string[] = JSON.parse(getConfig("audio_languages") ?? "[]"); + const audioLanguages = getAudioLanguages(); const analysis = analyzeItem( { original_language: item.original_language, needs_review: item.needs_review, container: item.container }, streams, @@ -186,7 +200,7 @@ function recomputePlanAfterToggle(db: ReturnType, itemId: number): }[]; const origLang = item.original_language ? normalizeLanguage(item.original_language) : null; - const audioLanguages: string[] = JSON.parse(getConfig("audio_languages") ?? "[]"); + const audioLanguages = getAudioLanguages(); // Re-assign target_index based on current actions const decWithIdx = decisions.map((d) => ({ diff --git a/server/api/scan.ts b/server/api/scan.ts index f906657..4c83543 100644 --- a/server/api/scan.ts +++ b/server/api/scan.ts @@ -25,6 +25,16 @@ function currentScanLimit(): number | null { return v ? Number(v) : null; } +function parseLanguageList(raw: string | null, fallback: string[]): string[] { + if (!raw) return fallback; + try { + const parsed = JSON.parse(raw); + return Array.isArray(parsed) ? parsed.filter((v): v is string => typeof v === "string") : fallback; + } catch { + return fallback; + } +} + // ─── Status ─────────────────────────────────────────────────────────────────── app.get("/", (c) => { @@ -140,8 +150,8 @@ async function runScan(limit: number | null = null): Promise { const cfg = getAllConfig(); const jellyfinCfg = { url: cfg.jellyfin_url, apiKey: cfg.jellyfin_api_key, userId: cfg.jellyfin_user_id }; - const subtitleLanguages: string[] = JSON.parse(cfg.subtitle_languages ?? '["eng","deu","spa"]'); - const audioLanguages: string[] = JSON.parse(cfg.audio_languages ?? "[]"); + const subtitleLanguages = parseLanguageList(cfg.subtitle_languages ?? null, ["eng", "deu", "spa"]); + const audioLanguages = parseLanguageList(cfg.audio_languages ?? null, []); const radarrEnabled = cfg.radarr_enabled === "1"; const sonarrEnabled = cfg.sonarr_enabled === "1"; diff --git a/src/features/dashboard/DashboardPage.tsx b/src/features/dashboard/DashboardPage.tsx index 35bad5a..5a03671 100644 --- a/src/features/dashboard/DashboardPage.tsx +++ b/src/features/dashboard/DashboardPage.tsx @@ -43,7 +43,7 @@ export function DashboardPage() { .then((d) => { setData(d); setLoading(false); - if (!d.setupComplete) navigate({ to: "/setup" }); + if (!d.setupComplete) navigate({ to: "/settings" }); }) .catch(() => setLoading(false)); }, [navigate]); diff --git a/src/features/execute/ExecutePage.tsx b/src/features/execute/ExecutePage.tsx index 85735ea..f95481f 100644 --- a/src/features/execute/ExecutePage.tsx +++ b/src/features/execute/ExecutePage.tsx @@ -1,5 +1,5 @@ import { Link, useNavigate, useSearch } from "@tanstack/react-router"; -import { useEffect, useRef, useState } from "react"; +import { Fragment, useCallback, useEffect, useRef, useState } from "react"; import { Badge } from "~/shared/components/ui/badge"; import { Button } from "~/shared/components/ui/button"; import { FilterTabs } from "~/shared/components/ui/filter-tabs"; @@ -33,7 +33,7 @@ function itemName(job: Job, item: MediaItem | null): string { } function jobTypeLabel(job: Job): string { - return job.job_type === "subtitle" ? "ST Extract" : "Audio Mod"; + return job.job_type === "transcode" ? "Audio Transcode" : "Audio Remux"; } // Module-level cache for instant tab switching @@ -50,28 +50,31 @@ export function ExecutePage() { const esRef = useRef(null); const reloadTimerRef = useRef | null>(null); - const load = (f?: string) => { - const key = f ?? filter; - const cached = cache.get(key); - if (cached && key === filter) { - setData(cached); - setLoading(false); - } else if (key === filter) { - setLoading(true); - } - api - .get(`/api/execute?filter=${key}`) - .then((d) => { - cache.set(key, d); - if (key === filter) { - setData(d); - setLoading(false); - } - }) - .catch(() => { - if (key === filter) setLoading(false); - }); - }; + const load = useCallback( + (f?: string) => { + const key = f ?? filter; + const cached = cache.get(key); + if (cached && key === filter) { + setData(cached); + setLoading(false); + } else if (key === filter) { + setLoading(true); + } + api + .get(`/api/execute?filter=${key}`) + .then((d) => { + cache.set(key, d); + if (key === filter) { + setData(d); + setLoading(false); + } + }) + .catch(() => { + if (key === filter) setLoading(false); + }); + }, + [filter], + ); useEffect(() => { load(); @@ -245,7 +248,7 @@ export function ExecutePage() { const showCmd = cmdVisible.has(job.id); return ( - <> + {job.id} @@ -264,7 +267,7 @@ export function ExecutePage() { - {jobTypeLabel(job)} + {jobTypeLabel(job)} {job.status} @@ -315,7 +318,7 @@ export function ExecutePage() { )} - + ); })} diff --git a/src/features/paths/PathsPage.tsx b/src/features/paths/PathsPage.tsx index 18ea5ca..0de9528 100644 --- a/src/features/paths/PathsPage.tsx +++ b/src/features/paths/PathsPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { Badge } from "~/shared/components/ui/badge"; import { Button } from "~/shared/components/ui/button"; import { api } from "~/shared/lib/api"; @@ -15,7 +15,7 @@ export function PathsPage() { const [paths, setPaths] = useState(cache ?? []); const [loading, setLoading] = useState(cache === null); - const load = () => { + const load = useCallback(() => { setLoading(true); api .get<{ paths: PathInfo[] }>("/api/paths") @@ -24,7 +24,7 @@ export function PathsPage() { setPaths(d.paths); }) .finally(() => setLoading(false)); - }; + }, []); useEffect(() => { if (cache === null) load(); diff --git a/src/features/review/AudioDetailPage.tsx b/src/features/review/AudioDetailPage.tsx index 7977d7e..a7c1803 100644 --- a/src/features/review/AudioDetailPage.tsx +++ b/src/features/review/AudioDetailPage.tsx @@ -1,5 +1,5 @@ import { Link, useParams } from "@tanstack/react-router"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { Alert } from "~/shared/components/ui/alert"; import { Badge } from "~/shared/components/ui/badge"; import { Button } from "~/shared/components/ui/button"; @@ -213,14 +213,17 @@ export function AudioDetailPage() { const [loading, setLoading] = useState(true); const [rescanning, setRescanning] = useState(false); - const load = () => - api - .get(`/api/review/${id}`) - .then((d) => { - setData(d); - setLoading(false); - }) - .catch(() => setLoading(false)); + const load = useCallback( + () => + api + .get(`/api/review/${id}`) + .then((d) => { + setData(d); + setLoading(false); + }) + .catch(() => setLoading(false)), + [id], + ); useEffect(() => { load(); }, [load]); diff --git a/src/features/scan/ScanPage.tsx b/src/features/scan/ScanPage.tsx index 7d132ef..7f5b453 100644 --- a/src/features/scan/ScanPage.tsx +++ b/src/features/scan/ScanPage.tsx @@ -112,7 +112,7 @@ export function ScanPage() { [], ); - const load = async () => { + const load = useCallback(async () => { const s = await api.get("/api/scan"); setStatus(s); setProgressScanned(s.progress.scanned); @@ -121,7 +121,7 @@ export function ScanPage() { setStatusLabel(s.running ? "Scan in progress…" : "Scan idle"); if (s.scanLimit != null) setLimit(String(s.scanLimit)); setLog(s.recentItems.map((i) => ({ name: i.name, type: i.type, status: i.scan_status, file: i.file_path }))); - }; + }, []); useEffect(() => { load(); diff --git a/src/features/setup/SetupPage.tsx b/src/features/setup/SetupPage.tsx index cb6f5fb..78daefd 100644 --- a/src/features/setup/SetupPage.tsx +++ b/src/features/setup/SetupPage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { Button } from "~/shared/components/ui/button"; import { Input } from "~/shared/components/ui/input"; import { Select } from "~/shared/components/ui/select"; @@ -242,23 +242,23 @@ export function SetupPage() { const [subSaved, setSubSaved] = useState(""); const [audLangs, setAudLangs] = useState([]); const [audSaved, setAudSaved] = useState(""); - const [langsLoaded, setLangsLoaded] = useState(false); + const langsLoadedRef = useRef(false); - const load = () => { + const load = useCallback(() => { if (!setupCache) setLoading(true); api .get("/api/setup") .then((d) => { setupCache = d; setData(d); - if (!langsLoaded) { + if (!langsLoadedRef.current) { setSubLangs(JSON.parse(d.config.subtitle_languages ?? '["eng","deu","spa"]')); setAudLangs(JSON.parse(d.config.audio_languages ?? "[]")); - setLangsLoaded(true); + langsLoadedRef.current = true; } }) .finally(() => setLoading(false)); - }; + }, []); useEffect(() => { load(); }, [load]); @@ -268,9 +268,15 @@ export function SetupPage() { const { config: cfg, envLocked: envLockedArr } = data; const locked = new Set(envLockedArr); - const saveJellyfin = (url: string, apiKey: string) => api.post("/api/setup/jellyfin", { url, api_key: apiKey }); - const saveRadarr = (url: string, apiKey: string) => api.post("/api/setup/radarr", { url, api_key: apiKey }); - const saveSonarr = (url: string, apiKey: string) => api.post("/api/setup/sonarr", { url, api_key: apiKey }); + const saveJellyfin = async (url: string, apiKey: string): Promise => { + await api.post("/api/setup/jellyfin", { url, api_key: apiKey }); + }; + const saveRadarr = async (url: string, apiKey: string): Promise => { + await api.post("/api/setup/radarr", { url, api_key: apiKey }); + }; + const saveSonarr = async (url: string, apiKey: string): Promise => { + await api.post("/api/setup/sonarr", { url, api_key: apiKey }); + }; const saveSubtitleLangs = async () => { await api.post("/api/setup/subtitle-languages", { langs: subLangs }); diff --git a/src/features/subtitles/SubtitleDetailPage.tsx b/src/features/subtitles/SubtitleDetailPage.tsx index c5fd20d..1e78406 100644 --- a/src/features/subtitles/SubtitleDetailPage.tsx +++ b/src/features/subtitles/SubtitleDetailPage.tsx @@ -1,5 +1,5 @@ import { Link, useParams } from "@tanstack/react-router"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { Alert } from "~/shared/components/ui/alert"; import { Badge } from "~/shared/components/ui/badge"; import { Button } from "~/shared/components/ui/button"; @@ -219,14 +219,17 @@ export function SubtitleDetailPage() { const [loading, setLoading] = useState(true); const [rescanning, setRescanning] = useState(false); - const load = () => - api - .get(`/api/subtitles/${id}`) - .then((d) => { - setData(d); - setLoading(false); - }) - .catch(() => setLoading(false)); + const load = useCallback( + () => + api + .get(`/api/subtitles/${id}`) + .then((d) => { + setData(d); + setLoading(false); + }) + .catch(() => setLoading(false)), + [id], + ); useEffect(() => { load(); }, [load]); diff --git a/src/features/subtitles/SubtitleListPage.tsx b/src/features/subtitles/SubtitleListPage.tsx index adc3b67..f051ca2 100644 --- a/src/features/subtitles/SubtitleListPage.tsx +++ b/src/features/subtitles/SubtitleListPage.tsx @@ -1,5 +1,5 @@ import type React from "react"; -import { useEffect, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; import { Button } from "~/shared/components/ui/button"; import { api } from "~/shared/lib/api"; import { langName } from "~/shared/lib/lang"; @@ -234,7 +234,7 @@ export function SubtitleListPage() { const [summary, setSummary] = useState(summaryCache); const [loading, setLoading] = useState(summaryCache === null); - const loadSummary = () => { + const loadSummary = useCallback(() => { if (!summaryCache) setLoading(true); api .get("/api/subtitles/summary") @@ -244,7 +244,7 @@ export function SubtitleListPage() { }) .catch(() => {}) .finally(() => setLoading(false)); - }; + }, []); useEffect(() => { loadSummary();