mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-07-28 04:49:49 -07:00
fix(stats): address CodeRabbit review on delete and vocabulary paths
- Plan lexical removals inside each delete transaction. The plan drives a subtraction now rather than a recompute, so a snapshot taken before BEGIN IMMEDIATE could subtract totals that no longer match the rows removed, and only drift reaching <= 0 self-heals. - Await the active video's pending anime metadata before deleteAnime's guard, so a delete issued while the title association is still being parsed can't slip past and let the late update recreate the anime row. - Order vocabulary pages by (frequency, id). The oversample loop re-runs with a growing OFFSET, and tied frequencies gave no stable order, so rows could be skipped or repeated across pages. - Clear the delete error when a retry starts, so cancelling leaves no stale failure on screen. - Seed occurrences, kanji, rollups and cover art in the deleteAnime test; the empty-table assertions were passing against empty tables. - Rename both keyframes to kebab-case for stylelint keyframes-name-pattern.
This commit is contained in:
@@ -1466,6 +1466,64 @@ test('deleteVideo ignores the currently active video and keeps new writes flusha
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Attach the derived rows a real session would produce to every recorded line
|
||||
* of `animeId`: word/kanji entries and their occurrences, monthly rollups, and
|
||||
* cover art backed by a shared blob.
|
||||
*/
|
||||
function seedDerivedAnimeData(db: DatabaseSync, animeId: number): void {
|
||||
const lines = db
|
||||
.prepare(
|
||||
'SELECT line_id AS lineId, CREATED_DATE AS seenMs FROM imm_subtitle_lines WHERE anime_id = ?',
|
||||
)
|
||||
.all(animeId) as Array<{ lineId: number; seenMs: number }>;
|
||||
assert.ok(lines.length > 0, 'expected recorded subtitle lines to decorate');
|
||||
|
||||
db.prepare(
|
||||
`INSERT INTO imm_words(id, headword, word, reading, part_of_speech, pos1, first_seen, last_seen, frequency)
|
||||
VALUES (9001, '天気', '天気', 'てんき', 'noun', '名詞', 0, 0, 0)`,
|
||||
).run();
|
||||
db.prepare(
|
||||
`INSERT INTO imm_kanji(id, kanji, first_seen, last_seen, frequency) VALUES (9101, '気', 0, 0, 0)`,
|
||||
).run();
|
||||
|
||||
const insertWordOccurrence = db.prepare(
|
||||
'INSERT INTO imm_word_line_occurrences(line_id, word_id, occurrence_count, seen_ms) VALUES (?, 9001, 1, ?)',
|
||||
);
|
||||
const insertKanjiOccurrence = db.prepare(
|
||||
'INSERT INTO imm_kanji_line_occurrences(line_id, kanji_id, occurrence_count, seen_ms) VALUES (?, 9101, 1, ?)',
|
||||
);
|
||||
for (const line of lines) {
|
||||
insertWordOccurrence.run(line.lineId, line.seenMs);
|
||||
insertKanjiOccurrence.run(line.lineId, line.seenMs);
|
||||
}
|
||||
db.prepare(
|
||||
`UPDATE imm_words SET frequency = ?, first_seen = ?, last_seen = ? WHERE id = 9001`,
|
||||
).run(lines.length, 0, 0);
|
||||
|
||||
const videoIds = (
|
||||
db
|
||||
.prepare('SELECT video_id AS videoId FROM imm_videos WHERE anime_id = ?')
|
||||
.all(animeId) as Array<{ videoId: number }>
|
||||
).map((row) => row.videoId);
|
||||
const insertMonthlyRollup = db.prepare(
|
||||
`INSERT INTO imm_monthly_rollups(rollup_month, video_id, total_sessions, CREATED_DATE, LAST_UPDATE_DATE)
|
||||
VALUES (202401, ?, 1, '0', '0')`,
|
||||
);
|
||||
const insertArt = db.prepare(
|
||||
`INSERT INTO imm_media_art(video_id, anilist_id, cover_url, cover_blob, cover_blob_hash, fetched_at_ms, CREATED_DATE, LAST_UPDATE_DATE)
|
||||
VALUES (?, 4242, 'https://example.test/cover.jpg', NULL, 'deadbeef', '0', '0', '0')`,
|
||||
);
|
||||
db.prepare(
|
||||
`INSERT INTO imm_cover_art_blobs(blob_hash, cover_blob, CREATED_DATE, LAST_UPDATE_DATE)
|
||||
VALUES ('deadbeef', X'FFD8FFD9', '0', '0')`,
|
||||
).run();
|
||||
for (const videoId of videoIds) {
|
||||
insertMonthlyRollup.run(videoId);
|
||||
insertArt.run(videoId);
|
||||
}
|
||||
}
|
||||
|
||||
test('deleteAnime removes every episode, session and library row for the title', async () => {
|
||||
const dbPath = makeDbPath();
|
||||
let tracker: ImmersionTrackerService | null = null;
|
||||
@@ -1492,6 +1550,29 @@ test('deleteAnime removes every episode, session and library row for the title',
|
||||
)?.anime_id;
|
||||
assert.ok(animeId);
|
||||
|
||||
// The tokenizer does not run in this harness, so attach vocabulary, kanji,
|
||||
// rollups and cover art to the recorded lines by hand. Without them the
|
||||
// "everything is gone" assertions below would pass against empty tables.
|
||||
seedDerivedAnimeData(privateApi.db, animeId);
|
||||
|
||||
const countOf = (sql: string): number =>
|
||||
(privateApi.db.prepare(sql).get() as { total: number }).total;
|
||||
for (const table of [
|
||||
'imm_words',
|
||||
'imm_kanji',
|
||||
'imm_word_line_occurrences',
|
||||
'imm_kanji_line_occurrences',
|
||||
'imm_daily_rollups',
|
||||
'imm_monthly_rollups',
|
||||
'imm_media_art',
|
||||
'imm_cover_art_blobs',
|
||||
]) {
|
||||
assert.ok(
|
||||
countOf(`SELECT COUNT(*) AS total FROM ${table}`) > 0,
|
||||
`precondition: ${table} should hold rows before the delete`,
|
||||
);
|
||||
}
|
||||
|
||||
const libraryBefore = await tracker.getAnimeLibrary();
|
||||
assert.equal(libraryBefore.length, 1);
|
||||
assert.equal(libraryBefore[0]?.episodeCount, 2);
|
||||
@@ -1501,16 +1582,28 @@ test('deleteAnime removes every episode, session and library row for the title',
|
||||
const libraryAfter = await tracker.getAnimeLibrary();
|
||||
assert.equal(libraryAfter.length, 0);
|
||||
|
||||
const countOf = (sql: string): number =>
|
||||
(privateApi.db.prepare(sql).get() as { total: number }).total;
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_anime'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_lifetime_anime'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_videos'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_sessions'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_subtitle_lines'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_daily_rollups'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_lifetime_media'), 0);
|
||||
assert.equal(countOf('SELECT COUNT(*) AS total FROM imm_words'), 0);
|
||||
for (const table of [
|
||||
'imm_anime',
|
||||
'imm_lifetime_anime',
|
||||
'imm_videos',
|
||||
'imm_sessions',
|
||||
'imm_subtitle_lines',
|
||||
'imm_daily_rollups',
|
||||
'imm_monthly_rollups',
|
||||
'imm_lifetime_media',
|
||||
'imm_words',
|
||||
'imm_kanji',
|
||||
'imm_word_line_occurrences',
|
||||
'imm_kanji_line_occurrences',
|
||||
'imm_media_art',
|
||||
'imm_cover_art_blobs',
|
||||
]) {
|
||||
assert.equal(
|
||||
countOf(`SELECT COUNT(*) AS total FROM ${table}`),
|
||||
0,
|
||||
`${table} should be empty after deleting the only title`,
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
tracker?.destroy();
|
||||
cleanupDbPath(dbPath);
|
||||
|
||||
@@ -735,6 +735,14 @@ export class ImmersionTrackerService {
|
||||
}
|
||||
|
||||
async deleteAnime(animeId: number): Promise<void> {
|
||||
// The active video's anime link is assigned asynchronously after the title
|
||||
// is parsed, so a guard reading imm_videos too early sees a null and lets
|
||||
// the delete through — then the late update recreates the anime row.
|
||||
const pendingVideoId = this.sessionState?.videoId;
|
||||
if (pendingVideoId !== undefined) {
|
||||
await this.pendingAnimeMetadataUpdates.get(pendingVideoId);
|
||||
}
|
||||
|
||||
const activeVideoId = this.sessionState?.videoId;
|
||||
if (activeVideoId !== undefined) {
|
||||
const activeAnime = this.db
|
||||
|
||||
@@ -125,7 +125,7 @@ export function getVocabularyStats(
|
||||
frequency, frequency_rank, first_seen, last_seen
|
||||
FROM imm_words
|
||||
${whereClause}
|
||||
ORDER BY frequency DESC
|
||||
ORDER BY frequency DESC, id
|
||||
LIMIT ? OFFSET ?
|
||||
)
|
||||
SELECT p.id AS wordId, p.headword, p.word, p.reading,
|
||||
@@ -137,7 +137,7 @@ export function getVocabularyStats(
|
||||
LEFT JOIN imm_word_line_occurrences o ON o.word_id = p.id
|
||||
LEFT JOIN imm_subtitle_lines sl ON sl.line_id = o.line_id AND sl.anime_id IS NOT NULL
|
||||
GROUP BY p.id
|
||||
ORDER BY p.frequency DESC
|
||||
ORDER BY p.frequency DESC, p.id
|
||||
`);
|
||||
const visibleRows: VocabularyStatsRow[] = [];
|
||||
let offset = 0;
|
||||
|
||||
@@ -483,11 +483,14 @@ export function isVideoWatched(db: DatabaseSync, videoId: number): boolean {
|
||||
|
||||
export function deleteSession(db: DatabaseSync, sessionId: number): void {
|
||||
const sessionIds = [sessionId];
|
||||
const lexicalRemovals = planLexicalRemovalsForSessions(db, sessionIds);
|
||||
const affectedRollupGroups = getRollupGroupsForSessions(db, sessionIds);
|
||||
|
||||
db.exec('BEGIN IMMEDIATE');
|
||||
try {
|
||||
// Measured inside the write lock: the plan records what the delete removes,
|
||||
// and applying a plan taken against a different snapshot would subtract the
|
||||
// wrong totals from imm_words/imm_kanji.
|
||||
const lexicalRemovals = planLexicalRemovalsForSessions(db, sessionIds);
|
||||
const affectedRollupGroups = getRollupGroupsForSessions(db, sessionIds);
|
||||
deleteSessionsByIds(db, sessionIds);
|
||||
applyLexicalRemovals(db, lexicalRemovals);
|
||||
rebuildLifetimeSummariesInTransaction(db);
|
||||
@@ -501,11 +504,11 @@ export function deleteSession(db: DatabaseSync, sessionId: number): void {
|
||||
|
||||
export function deleteSessions(db: DatabaseSync, sessionIds: number[]): void {
|
||||
if (sessionIds.length === 0) return;
|
||||
const lexicalRemovals = planLexicalRemovalsForSessions(db, sessionIds);
|
||||
const affectedRollupGroups = getRollupGroupsForSessions(db, sessionIds);
|
||||
|
||||
db.exec('BEGIN IMMEDIATE');
|
||||
try {
|
||||
const lexicalRemovals = planLexicalRemovalsForSessions(db, sessionIds);
|
||||
const affectedRollupGroups = getRollupGroupsForSessions(db, sessionIds);
|
||||
deleteSessionsByIds(db, sessionIds);
|
||||
applyLexicalRemovals(db, lexicalRemovals);
|
||||
rebuildLifetimeSummariesInTransaction(db);
|
||||
@@ -526,30 +529,30 @@ export function deleteSessions(db: DatabaseSync, sessionIds: number[]): void {
|
||||
* pay for one full rebuild per episode.
|
||||
*/
|
||||
export function deleteAnime(db: DatabaseSync, animeId: number): void {
|
||||
const videoIds = (
|
||||
db.prepare('SELECT video_id FROM imm_videos WHERE anime_id = ?').all(animeId) as Array<{
|
||||
video_id: number;
|
||||
}>
|
||||
).map((row) => row.video_id);
|
||||
|
||||
const lexicalRemovals = planLexicalRemovalsForVideos(db, videoIds);
|
||||
const coverBlobHashes: string[] = [];
|
||||
const sessionIds: number[] = [];
|
||||
for (const videoId of videoIds) {
|
||||
const artRow = db
|
||||
.prepare('SELECT cover_blob_hash AS coverBlobHash FROM imm_media_art WHERE video_id = ?')
|
||||
.get(videoId) as { coverBlobHash: string | null } | undefined;
|
||||
if (artRow?.coverBlobHash) {
|
||||
coverBlobHashes.push(artRow.coverBlobHash);
|
||||
}
|
||||
const sessions = db
|
||||
.prepare('SELECT session_id FROM imm_sessions WHERE video_id = ?')
|
||||
.all(videoId) as Array<{ session_id: number }>;
|
||||
sessionIds.push(...sessions.map((session) => session.session_id));
|
||||
}
|
||||
|
||||
db.exec('BEGIN IMMEDIATE');
|
||||
try {
|
||||
const videoIds = (
|
||||
db.prepare('SELECT video_id FROM imm_videos WHERE anime_id = ?').all(animeId) as Array<{
|
||||
video_id: number;
|
||||
}>
|
||||
).map((row) => row.video_id);
|
||||
|
||||
const lexicalRemovals = planLexicalRemovalsForVideos(db, videoIds);
|
||||
const coverBlobHashes: string[] = [];
|
||||
const sessionIds: number[] = [];
|
||||
for (const videoId of videoIds) {
|
||||
const artRow = db
|
||||
.prepare('SELECT cover_blob_hash AS coverBlobHash FROM imm_media_art WHERE video_id = ?')
|
||||
.get(videoId) as { coverBlobHash: string | null } | undefined;
|
||||
if (artRow?.coverBlobHash) {
|
||||
coverBlobHashes.push(artRow.coverBlobHash);
|
||||
}
|
||||
const sessions = db
|
||||
.prepare('SELECT session_id FROM imm_sessions WHERE video_id = ?')
|
||||
.all(videoId) as Array<{ session_id: number }>;
|
||||
sessionIds.push(...sessions.map((session) => session.session_id));
|
||||
}
|
||||
|
||||
deleteSessionsByIds(db, sessionIds);
|
||||
const deleteLinesStmt = db.prepare('DELETE FROM imm_subtitle_lines WHERE video_id = ?');
|
||||
const deleteDailyStmt = db.prepare('DELETE FROM imm_daily_rollups WHERE video_id = ?');
|
||||
@@ -578,22 +581,22 @@ export function deleteAnime(db: DatabaseSync, animeId: number): void {
|
||||
}
|
||||
|
||||
export function deleteVideo(db: DatabaseSync, videoId: number): void {
|
||||
const artRow = db
|
||||
.prepare(
|
||||
`
|
||||
db.exec('BEGIN IMMEDIATE');
|
||||
try {
|
||||
const artRow = db
|
||||
.prepare(
|
||||
`
|
||||
SELECT cover_blob_hash AS coverBlobHash
|
||||
FROM imm_media_art
|
||||
WHERE video_id = ?
|
||||
`,
|
||||
)
|
||||
.get(videoId) as { coverBlobHash: string | null } | undefined;
|
||||
const lexicalRemovals = planLexicalRemovalsForVideos(db, [videoId]);
|
||||
const sessions = db
|
||||
.prepare('SELECT session_id FROM imm_sessions WHERE video_id = ?')
|
||||
.all(videoId) as Array<{ session_id: number }>;
|
||||
)
|
||||
.get(videoId) as { coverBlobHash: string | null } | undefined;
|
||||
const lexicalRemovals = planLexicalRemovalsForVideos(db, [videoId]);
|
||||
const sessions = db
|
||||
.prepare('SELECT session_id FROM imm_sessions WHERE video_id = ?')
|
||||
.all(videoId) as Array<{ session_id: number }>;
|
||||
|
||||
db.exec('BEGIN IMMEDIATE');
|
||||
try {
|
||||
deleteSessionsByIds(
|
||||
db,
|
||||
sessions.map((session) => session.session_id),
|
||||
|
||||
@@ -165,6 +165,9 @@ export function AnimeDetailView({
|
||||
const handleDeleteAnime = async () => {
|
||||
if (isDeletingAnimeRef.current) return;
|
||||
isDeletingAnimeRef.current = true;
|
||||
// Cleared up front so cancelling a retry doesn't leave the previous
|
||||
// attempt's failure on screen.
|
||||
setDeleteError(null);
|
||||
let confirmed = false;
|
||||
try {
|
||||
confirmed = await confirmAnimeDelete(detail.canonicalTitle, detail.episodeCount);
|
||||
|
||||
@@ -80,7 +80,7 @@ body.overlay-mode #root {
|
||||
}
|
||||
|
||||
/* Tab content entrance animation */
|
||||
@keyframes fadeSlideIn {
|
||||
@keyframes fade-slide-in {
|
||||
from {
|
||||
opacity: 0;
|
||||
transform: translateY(6px);
|
||||
@@ -92,11 +92,11 @@ body.overlay-mode #root {
|
||||
}
|
||||
|
||||
.animate-fade-in {
|
||||
animation: fadeSlideIn 0.25s ease-out;
|
||||
animation: fade-slide-in 0.25s ease-out;
|
||||
}
|
||||
|
||||
/* Indeterminate progress sweep for the global delete indicator */
|
||||
@keyframes indeterminateSweep {
|
||||
@keyframes indeterminate-sweep {
|
||||
from {
|
||||
transform: translateX(-100%);
|
||||
}
|
||||
@@ -106,5 +106,5 @@ body.overlay-mode #root {
|
||||
}
|
||||
|
||||
.animate-indeterminate {
|
||||
animation: indeterminateSweep 1.1s ease-in-out infinite;
|
||||
animation: indeterminate-sweep 1.1s ease-in-out infinite;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user