fix(stats): fix data loss and error handling in anime merge/move

- flush pending telemetry before merge/move so in-progress session time isn't dropped from lifetime totals
- repoint subtitle lines by video_id instead of anime_id so lines recorded before the async title parse assigns a link aren't stranded
- stop absorbing metadata into the target when moving an episode out of an emptied entry (a move isn't a same-show claim)
- return 404 only for missing episode/target, not storage failures; return 404 when a merge folds nothing
- keep AnimeMergeDialog open while a merge is in flight instead of letting dismiss race the request
- surface library load failures in LibraryEntryPicker instead of showing an empty list
This commit is contained in:
2026-08-10 23:06:00 -07:00
parent 5938095d92
commit d6e6e29b5e
9 changed files with 164 additions and 17 deletions
@@ -3109,6 +3109,26 @@ Aligned English subtitle
});
});
it('POST /api/stats/anime/:animeId/merge reports a merge that folded nothing as 404', async () => {
const app = createStatsApp(
createMockTracker({
mergeAnime: async (targetAnimeId: number) => ({
survivingAnimeId: targetAnimeId,
mergedAnimeIds: [],
movedVideos: 0,
}),
} as Partial<ImmersionTrackerService>),
);
const res = await app.request('/api/stats/anime/7/merge', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: '{"sourceAnimeIds":[8]}',
});
assert.equal(res.status, 404);
});
it('PATCH /api/stats/media/:videoId/anime reports an unknown target as 404', async () => {
const app = createStatsApp(
createMockTracker({
@@ -3127,6 +3147,25 @@ Aligned English subtitle
assert.equal(res.status, 404);
});
it('PATCH /api/stats/media/:videoId/anime does not disguise storage failures as 404', async () => {
const app = createStatsApp(
createMockTracker({
moveVideoToAnime: async () => {
throw new Error('database is locked');
},
} as Partial<ImmersionTrackerService>),
);
const res = await app.request('/api/stats/media/12/anime', {
method: 'PATCH',
headers: { 'Content-Type': 'application/json' },
body: '{"animeId":7}',
});
assert.notEqual(res.status, 404);
assert.equal(res.status >= 500, true);
});
it('POST /api/stats/anki/browse returns 400 for missing noteId', async () => {
const app = createStatsApp(createMockTracker());
const res = await app.request('/api/stats/anki/browse', { method: 'POST' });
@@ -772,11 +772,18 @@ export class ImmersionTrackerService {
if (pendingVideoId !== undefined) {
await this.pendingAnimeMetadataUpdates.get(pendingVideoId);
}
// Both of these rebuild the lifetime summaries, which recompute from the
// database: queued telemetry has to land first or the active session's
// watch time is dropped from the merged totals.
this.flushTelemetry(true);
this.flushNow();
return mergeAnimeRecords(this.db, targetAnimeId, sourceAnimeIds);
}
async moveVideoToAnime(videoId: number, targetAnimeId: number): Promise<VideoMoveSummary> {
await this.pendingAnimeMetadataUpdates.get(videoId);
this.flushTelemetry(true);
this.flushNow();
return moveVideoToAnimeQuery(this.db, videoId, targetAnimeId);
}
@@ -157,6 +157,33 @@ test('mergeAnimeRecords folds episodes, lines and lifetime totals into the targe
});
});
test('mergeAnimeRecords repoints subtitle lines recorded before the anime link landed', () => {
withDb((db) => {
insertAnime(db, { animeId: 1, key: 'show', title: 'Show' });
insertAnime(db, { animeId: 2, key: 'show season 1', title: 'Show Season 1' });
insertEpisode(db, { videoId: 1, animeId: 1 });
insertEpisode(db, { videoId: 2, animeId: 2, season: 1 });
// Lines are written with the video's anime_id at the time, which is NULL
// until the async title parse assigns one.
db.prepare(
`INSERT INTO imm_subtitle_lines(session_id, video_id, anime_id, line_index, text, CREATED_DATE, LAST_UPDATE_DATE)
VALUES (2, 2, NULL, 2, 'unlinked line', ?, ?)`,
).run(BASE_MS, BASE_MS);
mergeAnimeRecords(db, 1, [2]);
assert.equal(lineAnimeIds(db, 1), 3);
const orphaned = Number(
(
db
.prepare('SELECT COUNT(*) AS total FROM imm_subtitle_lines WHERE anime_id IS NULL')
.get() as { total: number }
).total,
);
assert.equal(orphaned, 0);
});
});
test('mergeAnimeRecords inherits metadata the target is missing without clobbering its own', () => {
withDb((db) => {
insertAnime(db, { animeId: 1, key: 'show', title: 'Show', titleRomaji: 'Shou' });
@@ -215,6 +242,27 @@ test('moveVideoToAnime moves one episode and prunes the emptied entry', () => {
.prepare('SELECT total_active_ms AS activeMs FROM imm_lifetime_anime WHERE anime_id = 1')
.get() as { activeMs: number };
assert.equal(lifetime.activeMs, 6000);
// The stray entry's AniList link is dropped, not inherited: a move makes no
// claim that the two entries are the same show.
const target = db
.prepare('SELECT anilist_id AS anilistId FROM imm_anime WHERE anime_id = 1')
.get() as { anilistId: number | null };
assert.equal(target.anilistId, null);
});
});
test('moveVideoToAnime is a no-op when the episode is already in the target entry', () => {
withDb((db) => {
insertAnime(db, { animeId: 1, key: 'show', title: 'Show' });
insertEpisode(db, { videoId: 1, animeId: 1 });
const summary = moveVideoToAnime(db, 1, 1);
assert.equal(summary.targetAnimeId, 1);
assert.equal(summary.previousAnimeId, 1);
assert.equal(summary.removedPreviousAnime, false);
assert.deepEqual(animeIds(db), [1]);
assert.equal(videoAnimeId(db, 1), 1);
});
});
@@ -3,6 +3,9 @@ import { rebuildLifetimeSummariesInTransaction } from './lifetime';
import { toDbTimestamp } from './query-shared';
import { nowMs } from './time';
/** Thrown when a move names an episode or destination entry that is not there. */
export const UNKNOWN_MOVE_TARGET_MESSAGE = 'Unknown episode or target library entry';
export interface AnimeMergeSummary {
/** Library entry that owns every moved episode once the merge finishes. */
survivingAnimeId: number;
@@ -161,11 +164,17 @@ export function mergeAnimeRecordsInTransaction(
}
const updatedAt = toDbTimestamp(nowMs());
const sourceVideosStmt = db.prepare(
'SELECT video_id AS videoId FROM imm_videos WHERE anime_id = ?',
);
const moveVideosStmt = db.prepare(
'UPDATE imm_videos SET anime_id = ?, LAST_UPDATE_DATE = ? WHERE anime_id = ?',
);
// Repointed per video rather than by anime_id: lines recorded before the
// async title parse assigns the link are stored with a NULL anime_id, and
// matching on the source id would strand them unattributed.
const moveLinesStmt = db.prepare(
'UPDATE imm_subtitle_lines SET anime_id = ?, LAST_UPDATE_DATE = ? WHERE anime_id = ?',
'UPDATE imm_subtitle_lines SET anime_id = ?, LAST_UPDATE_DATE = ? WHERE video_id = ?',
);
const dropLifetimeStmt = db.prepare('DELETE FROM imm_lifetime_anime WHERE anime_id = ?');
const dropAnimeStmt = db.prepare('DELETE FROM imm_anime WHERE anime_id = ?');
@@ -176,10 +185,15 @@ export function mergeAnimeRecordsInTransaction(
}
const sourceMetadata = readAnimeMetadata(db, sourceAnimeId);
const sourceVideoIds = (sourceVideosStmt.all(sourceAnimeId) as Array<{ videoId: number }>).map(
(row) => row.videoId,
);
const moved = moveVideosStmt.run(targetAnimeId, updatedAt, sourceAnimeId) as {
changes: number;
};
moveLinesStmt.run(targetAnimeId, updatedAt, sourceAnimeId);
for (const videoId of sourceVideoIds) {
moveLinesStmt.run(targetAnimeId, updatedAt, videoId);
}
dropLifetimeStmt.run(sourceAnimeId);
dropAnimeStmt.run(sourceAnimeId);
absorbAnimeMetadata(db, targetAnimeId, sourceMetadata, updatedAt);
@@ -219,7 +233,7 @@ export function moveVideoToAnime(
.prepare('SELECT anime_id AS animeId FROM imm_videos WHERE video_id = ?')
.get(videoId) as { animeId: number | null } | null;
if (!videoRow || !animeExists(db, targetAnimeId)) {
throw new Error('Unknown episode or target library entry');
throw new Error(UNKNOWN_MOVE_TARGET_MESSAGE);
}
const previousAnimeId = videoRow.animeId;
@@ -239,10 +253,12 @@ export function moveVideoToAnime(
let removedPreviousAnime = false;
if (previousAnimeId !== null && !hasAnimeReferences(db, previousAnimeId)) {
const sourceMetadata = readAnimeMetadata(db, previousAnimeId);
// The emptied entry's metadata is deliberately dropped rather than
// absorbed. A move says "this episode belongs elsewhere", not "these are
// the same show", and the entry being emptied is usually a mis-parse
// whose AniList link would be wrong for the target.
db.prepare('DELETE FROM imm_lifetime_anime WHERE anime_id = ?').run(previousAnimeId);
db.prepare('DELETE FROM imm_anime WHERE anime_id = ?').run(previousAnimeId);
absorbAnimeMetadata(db, targetAnimeId, sourceMetadata, updatedAt);
removedPreviousAnime = true;
}
@@ -360,9 +360,11 @@ export function resolveAnimeAnilistConflict(
const summary = emptySummary(1);
summary.movedVideos = merge.movedVideos;
summary.deletedAnimeRows = merge.mergedAnimeIds.length;
summary.survivingAnimeId = survivingAnimeId;
if (merge.mergedAnimeIds.length > 0) {
summary.repaired = 1;
// Only reported once a row really absorbed the other, so callers never
// follow this to an anime id that was never written.
summary.survivingAnimeId = survivingAnimeId;
}
// Lifetime summaries are rebuilt by the caller off this summary, the same
// as the redistribution path below.
@@ -383,11 +385,16 @@ function canMergeAnilistConflict(
anilistId: number,
options: AnimeAnilistConflictOptions,
): boolean {
const targetRow = getAnimeRow(db, targetAnimeId);
if (!targetRow) {
// Nothing to merge with a row that no longer exists (a stale id from the
// caller); fall through to the redistribution path.
return false;
}
if (options.survivor !== 'target') {
// The target is the row about to disappear here, so an existing link of its
// own means this is a mis-resolution rather than a duplicate: leave it be.
const targetRow = getAnimeRow(db, targetAnimeId);
if (targetRow?.anilist_id != null && targetRow.anilist_id !== anilistId) {
if (targetRow.anilist_id != null && targetRow.anilist_id !== anilistId) {
return false;
}
}
@@ -1,5 +1,6 @@
import type { Hono } from 'hono';
import { statsJson } from '../../../types/stats-http-contract.js';
import { UNKNOWN_MOVE_TARGET_MESSAGE } from '../immersion-tracker/anime-merge.js';
import type { ImmersionTrackerService } from '../immersion-tracker-service.js';
import {
buildSentenceSearchOptions,
@@ -206,6 +207,9 @@ export function registerStatsLibraryRoutes(
const sourceAnimeIds = parsePositiveIdList(body?.sourceAnimeIds).filter((id) => id !== animeId);
if (sourceAnimeIds.length === 0) return c.body(null, 400);
const summary = await tracker.mergeAnime(animeId, sourceAnimeIds);
// Nothing folded means the target or every source was already gone, so the
// caller should not be told the merge succeeded.
if (summary.mergedAnimeIds.length === 0) return c.body(null, 404);
return c.json(
statsJson('mergeAnime', {
ok: true,
@@ -232,8 +236,13 @@ export function registerStatsLibraryRoutes(
removedPreviousAnime: summary.removedPreviousAnime,
}),
);
} catch {
return c.body(null, 404);
} catch (error) {
// Only a missing episode or entry is a 404; storage failures must not be
// reported to the caller as "not found".
if (error instanceof Error && error.message === UNKNOWN_MOVE_TARGET_MESSAGE) {
return c.body(null, 404);
}
throw error;
}
});
}
@@ -43,8 +43,18 @@ export function AnimeMergeDialog({ entries, onClose, onMerged }: AnimeMergeDialo
}
};
// Dismissing mid-request would leave the caller unaware of a merge that is
// still going to land, so the backdrop and close button are inert until it
// resolves.
const handleDismiss = () => {
if (!merging) onClose();
};
return (
<div className="fixed inset-0 z-50 flex items-start justify-center pt-[10vh]" onClick={onClose}>
<div
className="fixed inset-0 z-50 flex items-start justify-center pt-[10vh]"
onClick={handleDismiss}
>
<div className="absolute inset-0 bg-ctp-crust/70 backdrop-blur-[2px]" />
<div
className="relative bg-ctp-base border border-ctp-surface1 rounded-xl shadow-2xl w-full max-w-lg max-h-[70vh] flex flex-col animate-fade-in"
@@ -57,8 +67,9 @@ export function AnimeMergeDialog({ entries, onClose, onMerged }: AnimeMergeDialo
</h3>
<button
type="button"
onClick={onClose}
className="text-ctp-overlay2 hover:text-ctp-text text-lg leading-none"
onClick={handleDismiss}
disabled={merging}
className="text-ctp-overlay2 hover:text-ctp-text text-lg leading-none disabled:opacity-50"
>
{'✕'}
</button>
+2 -2
View File
@@ -205,7 +205,7 @@ export function EpisodeList({
setMoveError(null);
setMovingEpisode(ep);
}}
className={`w-5 h-5 rounded border border-ctp-surface2 text-transparent hover:border-ctp-blue/50 hover:text-ctp-blue hover:bg-ctp-blue/10 transition-colors text-xs flex items-center justify-center ${HOVER_REVEALED}`}
className={`w-5 h-5 rounded border border-ctp-surface2 text-transparent hover:border-ctp-blue/50 hover:text-ctp-blue focus-visible:text-ctp-blue hover:bg-ctp-blue/10 transition-colors text-xs flex items-center justify-center ${HOVER_REVEALED}`}
title="Move to another library entry"
aria-label="Move to another library entry"
>
@@ -217,7 +217,7 @@ export function EpisodeList({
e.stopPropagation();
void handleDeleteEpisode(ep.videoId, ep.canonicalTitle);
}}
className={`w-5 h-5 rounded border border-ctp-surface2 text-transparent hover:border-ctp-red/50 hover:text-ctp-red hover:bg-ctp-red/10 transition-colors text-xs flex items-center justify-center ${HOVER_REVEALED}`}
className={`w-5 h-5 rounded border border-ctp-surface2 text-transparent hover:border-ctp-red/50 hover:text-ctp-red focus-visible:text-ctp-red hover:bg-ctp-red/10 transition-colors text-xs flex items-center justify-center ${HOVER_REVEALED}`}
title="Delete episode"
aria-label="Delete episode"
>
@@ -25,6 +25,7 @@ export function LibraryEntryPicker({
onClose,
}: LibraryEntryPickerProps) {
const [entries, setEntries] = useState<AnimeLibraryItem[] | null>(null);
const [loadFailed, setLoadFailed] = useState(false);
const [query, setQuery] = useState(initialQuery);
const inputRef = useRef<HTMLInputElement>(null);
@@ -37,7 +38,11 @@ export function LibraryEntryPicker({
if (!cancelled) setEntries(data);
})
.catch(() => {
if (!cancelled) setEntries([]);
// Distinct from an empty library: telling the user "no other titles"
// when the request failed hides a retryable error.
if (cancelled) return;
setEntries([]);
setLoadFailed(true);
});
return () => {
cancelled = true;
@@ -84,7 +89,12 @@ export function LibraryEntryPicker({
<div className="flex-1 overflow-y-auto p-2">
{entries === null && <div className="text-xs text-ctp-overlay2 p-3">Loading...</div>}
{entries !== null && visible.length === 0 && (
{loadFailed && (
<div className="text-xs text-ctp-red p-3">
Could not load the library. Close this dialog and try again.
</div>
)}
{!loadFailed && entries !== null && visible.length === 0 && (
<div className="text-xs text-ctp-overlay2 p-3">No other titles</div>
)}
{visible.map((entry) => (