apply codex code review: fix useEffect refetch loops, dead routes, subtitle job_type leftovers
All checks were successful
Build and Push Docker Image / build (push) Successful in 36s

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<void> (matches the
  consumer signatures, fixes the latent TS error).
- DashboardPage: redirect target /setup doesn't exist; the route is
  /settings.
- ExecutePage: <>...</> fragment with two <tr> children had keys on the
  rows but not on the fragment → React reconciliation warning. Use
  <Fragment key>. 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.
This commit is contained in:
2026-04-13 12:01:57 +02:00
parent cafb3852a1
commit 1aafcb4972
10 changed files with 107 additions and 68 deletions

View File

@@ -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<typeof getDb>): Record<string, number> {
@@ -114,7 +128,7 @@ function reanalyze(db: ReturnType<typeof getDb>, 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<typeof getDb>, 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) => ({

View File

@@ -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<void> {
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";

View File

@@ -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]);

View File

@@ -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<EventSource | null>(null);
const reloadTimerRef = useRef<ReturnType<typeof setTimeout> | 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<ExecuteData>(`/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<ExecuteData>(`/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 (
<>
<Fragment key={job.id}>
<tr key={job.id} className="hover:bg-gray-50">
<td className="py-1.5 px-2 border-b border-gray-100 font-mono text-xs">{job.id}</td>
<td className="py-1.5 px-2 border-b border-gray-100">
@@ -264,7 +267,7 @@ export function ExecutePage() {
</div>
</td>
<td className="py-1.5 px-2 border-b border-gray-100 whitespace-nowrap">
<Badge variant={job.job_type === "subtitle" ? "noop" : "default"}>{jobTypeLabel(job)}</Badge>
<Badge variant={job.job_type === "transcode" ? "manual" : "noop"}>{jobTypeLabel(job)}</Badge>
</td>
<td className="py-1.5 px-2 border-b border-gray-100">
<Badge variant={job.status}>{job.status}</Badge>
@@ -315,7 +318,7 @@ export function ExecutePage() {
</td>
</tr>
)}
</>
</Fragment>
);
})}
</tbody>

View File

@@ -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<PathInfo[]>(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();

View File

@@ -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<DetailData>(`/api/review/${id}`)
.then((d) => {
setData(d);
setLoading(false);
})
.catch(() => setLoading(false));
const load = useCallback(
() =>
api
.get<DetailData>(`/api/review/${id}`)
.then((d) => {
setData(d);
setLoading(false);
})
.catch(() => setLoading(false)),
[id],
);
useEffect(() => {
load();
}, [load]);

View File

@@ -112,7 +112,7 @@ export function ScanPage() {
[],
);
const load = async () => {
const load = useCallback(async () => {
const s = await api.get<ScanStatus>("/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();

View File

@@ -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<string[]>([]);
const [audSaved, setAudSaved] = useState("");
const [langsLoaded, setLangsLoaded] = useState(false);
const langsLoadedRef = useRef(false);
const load = () => {
const load = useCallback(() => {
if (!setupCache) setLoading(true);
api
.get<SetupData>("/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<void> => {
await api.post("/api/setup/jellyfin", { url, api_key: apiKey });
};
const saveRadarr = async (url: string, apiKey: string): Promise<void> => {
await api.post("/api/setup/radarr", { url, api_key: apiKey });
};
const saveSonarr = async (url: string, apiKey: string): Promise<void> => {
await api.post("/api/setup/sonarr", { url, api_key: apiKey });
};
const saveSubtitleLangs = async () => {
await api.post("/api/setup/subtitle-languages", { langs: subLangs });

View File

@@ -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<DetailData>(`/api/subtitles/${id}`)
.then((d) => {
setData(d);
setLoading(false);
})
.catch(() => setLoading(false));
const load = useCallback(
() =>
api
.get<DetailData>(`/api/subtitles/${id}`)
.then((d) => {
setData(d);
setLoading(false);
})
.catch(() => setLoading(false)),
[id],
);
useEffect(() => {
load();
}, [load]);

View File

@@ -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<SummaryData | null>(summaryCache);
const [loading, setLoading] = useState(summaryCache === null);
const loadSummary = () => {
const loadSummary = useCallback(() => {
if (!summaryCache) setLoading(true);
api
.get<SummaryData>("/api/subtitles/summary")
@@ -244,7 +244,7 @@ export function SubtitleListPage() {
})
.catch(() => {})
.finally(() => setLoading(false));
};
}, []);
useEffect(() => {
loadSummary();