Fix M3: extract shared recordCompletion() for download-done side-effects

The completion side-effect (add to history unless private + markDownloaded on
the source MediaItem) was duplicated in applyEvent('done') and the preview
fake-ticker, and had already drifted (the preview copy omitted formatId/
formatHasAudio). Extracted one recordCompletion(item) helper that both call.

Also tightened a latent bug: the old applyEvent ran markDownloaded for any
'done' event with a mediaItemId, even when the item was canceled (the reducer
keeps canceled items as 'canceled'). recordCompletion now runs only when
status === 'completed', so a canceled collection item is no longer wrongly
marked downloaded (and will be retried on the next re-sync).

typecheck + 242 tests + eslint green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-30 15:36:13 -04:00
parent eb94e84707
commit c55e131fb0
2 changed files with 30 additions and 41 deletions
+1 -1
View File
@@ -223,7 +223,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
`history.ts`/`errorlog.ts`/`templates.ts` reimplement it inline. `history.ts`/`errorlog.ts`/`templates.ts` reimplement it inline.
- [x] **M2 — Remove dead code: `getDefaultFolder` / `download:default-folder`** (channel + - [x] **M2 — Remove dead code: `getDefaultFolder` / `download:default-folder`** (channel +
preload + handler + mock, zero callers). preload + handler + mock, zero callers).
- [ ] **M3 — Remove duplicated completion side-effect in `store/downloads.ts`** (history write + - [x] **M3 — Remove duplicated completion side-effect in `store/downloads.ts`** (history write +
`markDownloaded` in both `applyEvent('done')` and the preview ticker). Subsumed by C2. `markDownloaded` in both `applyEvent('done')` and the preview ticker). Subsumed by C2.
- [ ] **M4 — Decide queue persistence explicitly.** Scheduler/queue is renderer-memory only; - [ ] **M4 — Decide queue persistence explicitly.** Scheduler/queue is renderer-memory only;
`saved`/scheduled items don't survive a quit. Persist or document as a non-goal in code. `saved`/scheduled items don't survive a quit. Persist or document as a non-goal in code.
+29 -40
View File
@@ -252,6 +252,32 @@ const seed: DownloadItem[] = PREVIEW
] ]
: [] : []
// Side-effects to run exactly once when a download completes: record it to
// history (unless private) and mark its source MediaItem downloaded. Shared by
// the real `applyEvent('done')` path and the preview ticker so the two can't
// drift (M3).
function recordCompletion(item: DownloadItem): void {
if (!item.incognito) {
useHistory.getState().add({
id: item.id,
title: item.title,
channel: item.channel,
url: item.url,
kind: item.kind,
quality: item.quality,
formatId: item.formatId,
formatHasAudio: item.formatHasAudio,
filePath: item.filePath,
sizeLabel: item.sizeLabel,
thumbnail: item.thumbnail,
completedAt: Date.now()
})
}
if (item.mediaItemId) {
useSources.getState().markDownloaded(item.mediaItemId, item.filePath)
}
}
// UI-preview only: animate fake progress so the queue feels alive without yt-dlp. // UI-preview only: animate fake progress so the queue feels alive without yt-dlp.
function startFakeTicker( function startFakeTicker(
id: string, id: string,
@@ -286,25 +312,7 @@ function startFakeTicker(
if (done) { if (done) {
clearInterval(timer) clearInterval(timer)
const finished = get().items.find((i) => i.id === id) const finished = get().items.find((i) => i.id === id)
if (finished && !finished.incognito) { if (finished) recordCompletion(finished)
useHistory.getState().add({
id: finished.id,
title: finished.title,
channel: finished.channel,
url: finished.url,
kind: finished.kind,
quality: finished.quality,
filePath: finished.filePath,
sizeLabel: finished.sizeLabel,
thumbnail: finished.thumbnail,
completedAt: Date.now()
})
}
// Mirror the real applyEvent path so collection completions mark their
// source item downloaded in the preview too.
if (finished?.mediaItemId) {
useSources.getState().markDownloaded(finished.mediaItemId, finished.filePath)
}
get().pump() // a slot just freed -- promote the next queued item get().pump() // a slot just freed -- promote the next queued item
} }
}, 650) }, 650)
@@ -609,29 +617,10 @@ export const useDownloads = create<DownloadState>((set, get) => {
} }
}) })
})) }))
// Record completed downloads to history (unless this was a private download). // Record completion (history + source markDownloaded) via the shared helper.
if (ev.type === 'done') { if (ev.type === 'done') {
const item = get().items.find((i) => i.id === ev.id) const item = get().items.find((i) => i.id === ev.id)
if (item && item.status === 'completed' && !item.incognito) { if (item && item.status === 'completed') recordCompletion(item)
useHistory.getState().add({
id: item.id,
title: item.title,
channel: item.channel,
url: item.url,
kind: item.kind,
quality: item.quality,
formatId: item.formatId,
formatHasAudio: item.formatHasAudio,
filePath: item.filePath,
sizeLabel: item.sizeLabel,
thumbnail: item.thumbnail,
completedAt: Date.now()
})
}
// Persist media-manager completion so an incremental re-sync skips it.
if (item?.mediaItemId) {
useSources.getState().markDownloaded(item.mediaItemId, item.filePath)
}
} }
// A finished item frees a slot -- promote whatever is queued next. // A finished item frees a slot -- promote whatever is queued next.
if (ev.type === 'done' || ev.type === 'error') pump() if (ev.type === 'done' || ev.type === 'error') pump()