diff --git a/docs/design/batch-duplicate-review.md b/docs/design/batch-duplicate-review.md new file mode 100644 index 0000000..654ae3a --- /dev/null +++ b/docs/design/batch-duplicate-review.md @@ -0,0 +1,33 @@ +# Batch duplicate review + +## Problem + +Opening a diagnostic, opening each group again, scrolling to a separate repair +form, and applying one change at a time creates unnecessary friction and many +small backups. + +## Design + +Duplicate and cloud-conflict diagnostics now open as a sequential review queue. +Each group presents its candidates together with audio preview and Finder +controls. Choosing a keeper records the decision and advances immediately. +Uncertain groups can be skipped without changing them. + +All approved decisions are submitted as one dry-run plan. Apply creates one +backup containing every affected audio file plus the Serato metadata snapshot, +then installs all compatibility shortcuts as one rollback unit. Any validation +or filesystem failure prevents or rolls back the batch. + +## Edge cases + +- The same group cannot be submitted twice. +- A file cannot be both a keeper and a disposable file across choices. +- Every group is rescanned and revalidated before preview and apply. +- Skipped and unreviewed groups remain untouched. +- Revisiting a group preserves and visibly marks its selected keeper. + +## Tests + +- Two groups produce one backup and one restorable receipt. +- Batch preview reports combined changes without modifying files. +- Existing single-group repair remains a wrapper around the batch engine. diff --git a/serato_doctor/repair.py b/serato_doctor/repair.py index bccf41c..496b94a 100644 --- a/serato_doctor/repair.py +++ b/serato_doctor/repair.py @@ -83,8 +83,28 @@ def apply_duplicate_repair( backup_limit: Optional[int] = 10, ) -> RepairReceipt: """Create a complete rollback snapshot, then replace extras with symlinks.""" + return apply_duplicate_repair_batch((plan,), serato_root, backup_limit) + + +def apply_duplicate_repair_batch( + plans: Iterable[DuplicateRepairPlan], + serato_root: Path, + backup_limit: Optional[int] = 10, +) -> RepairReceipt: + """Apply several approved duplicate choices as one atomic backup.""" + plans = tuple(plans) + if not plans: + raise ValueError("Choose at least one duplicate group") if backup_limit is not None and backup_limit < 1: raise ValueError("Backup limit must be at least 1, or unlimited") + replaced_paths = tuple( + path for plan in plans for path in plan.replaced + ) + if len(set(replaced_paths)) != len(replaced_paths): + raise ValueError("The same file appears in more than one repair choice") + keepers = {plan.keeper for plan in plans} + if keepers.intersection(replaced_paths): + raise ValueError("A selected keeper cannot be removed by another choice") backup_root = serato_root.expanduser().resolve() / BACKUP_FOLDER backup = backup_root / _backup_name() files_root = backup / "files" @@ -93,35 +113,45 @@ def apply_duplicate_repair( entries = [] try: - for path in plan.replaced: - destination = files_root / _safe_backup_path(path) - destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(path, destination) - entries.append({"original": str(path), "backup": str(destination)}) - for path in plan.metadata_files: + for plan in plans: + for path in plan.replaced: + destination = files_root / _safe_backup_path(path) + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(path, destination) + entries.append( + { + "original": str(path), + "backup": str(destination), + "keeper": str(plan.keeper), + } + ) + for path in plans[0].metadata_files: relative = path.relative_to(serato_root.expanduser().resolve()) destination = metadata_root / relative destination.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(path, destination) manifest = { "created_at": datetime.now(timezone.utc).isoformat(), - "keeper": str(plan.keeper), + "keeper": str(plans[0].keeper), + "keepers": [str(plan.keeper) for plan in plans], + "choice_count": len(plans), "replaced": entries, "strategy": "symlink", } (backup / "manifest.json").write_text( json.dumps(manifest, indent=2), encoding="utf-8" ) - for path in plan.replaced: + for entry in entries: + path = Path(entry["original"]) path.unlink() - path.symlink_to(plan.keeper) + path.symlink_to(Path(entry["keeper"])) except Exception: _rollback_entries(entries) shutil.rmtree(backup, ignore_errors=True) raise rotate_backups(backup_root, backup_limit) - return RepairReceipt(backup, plan.keeper, plan.replaced) + return RepairReceipt(backup, plans[0].keeper, replaced_paths) def restore_backup(backup: Path) -> Tuple[Path, ...]: diff --git a/serato_doctor/web.py b/serato_doctor/web.py index 16f34f1..7b26505 100644 --- a/serato_doctor/web.py +++ b/serato_doctor/web.py @@ -26,6 +26,7 @@ from serato_doctor.scanner import scan_filesystem from serato_doctor.repair import ( BACKUP_FOLDER, apply_duplicate_repair, + apply_duplicate_repair_batch, list_backups, plan_duplicate_repair, restore_backup, @@ -324,6 +325,53 @@ def duplicate_repair( return result +def duplicate_repair_batch( + serato: Path, + music: Path, + choices: Iterable[dict], + backup_limit: Optional[int], + apply: bool = False, +) -> dict: + """Validate and preview or apply several keeper choices together.""" + serato = serato.expanduser().resolve() + music = music.expanduser().resolve() + if not serato.is_dir() or not music.is_dir(): + raise ValueError("Analyze the library again before repairing duplicates") + groups = find_duplicate_groups(scan_filesystem(music).tracks) + valid_groups = [ + {track.path.resolve() for track in group.tracks} for group in groups + ] + plans = [] + selected_groups = set() + for choice in choices: + requested = tuple(Path(value).expanduser().resolve() for value in choice["group_files"]) + group_key = frozenset(requested) + if set(requested) not in valid_groups: + raise ValueError("A duplicate group changed; analyze the library again") + if group_key in selected_groups: + raise ValueError("A duplicate group was selected more than once") + selected_groups.add(group_key) + plans.append( + plan_duplicate_repair(Path(choice["keeper"]), requested, serato) + ) + if not plans: + raise ValueError("Choose at least one duplicate group") + replaced = [str(path) for plan in plans for path in plan.replaced] + result = { + "choice_count": len(plans), + "replaced": replaced, + "metadata_backups": len(plans[0].metadata_files), + "strategy": "shortcut", + "database_v2_modified": False, + } + if apply: + receipt = apply_duplicate_repair_batch(plans, serato, backup_limit) + result.update({"applied": True, "backup": str(receipt.backup)}) + else: + result["applied"] = False + return result + + def backup_history(serato: Path) -> dict: serato = serato.expanduser().resolve() if not serato.is_dir(): @@ -354,6 +402,8 @@ class SeratoDoctorHandler(BaseHTTPRequestHandler): try: token = parse_qs(request.query)["token"][0] self._audio_response(_verified_audio(token)) + except (BrokenPipeError, ConnectionResetError): + return except (KeyError, IndexError, OSError, ValueError) as error: self._json_response(404, {"error": str(error)}) return @@ -379,6 +429,8 @@ class SeratoDoctorHandler(BaseHTTPRequestHandler): "/api/analyze", "/api/duplicates/preview", "/api/duplicates/apply", + "/api/duplicates/batch/preview", + "/api/duplicates/batch/apply", "/api/backups/restore", "/api/backups", "/api/reveal", @@ -412,6 +464,16 @@ class SeratoDoctorHandler(BaseHTTPRequestHandler): raise ValueError("That backup does not belong to this library") restored = restore_backup(backup) result = {"restored": [str(path) for path in restored]} + elif self.path.startswith("/api/duplicates/batch/"): + raw_limit = payload.get("backup_limit", 10) + backup_limit = None if raw_limit is None else int(raw_limit) + result = duplicate_repair_batch( + Path(payload["serato"]), + Path(payload["music"]), + payload["choices"], + backup_limit, + apply=self.path.endswith("/apply"), + ) else: raw_limit = payload.get("backup_limit", 10) backup_limit = None if raw_limit is None else int(raw_limit) diff --git a/serato_doctor/webui/app.js b/serato_doctor/webui/app.js index 64c2aa5..0126106 100644 --- a/serato_doctor/webui/app.js +++ b/serato_doctor/webui/app.js @@ -26,6 +26,7 @@ let latestAnalysis = null; let selectedDuplicateGroup = null; let previewedRepair = null; let latestBackup = null; +let reviewState = null; function expandHome(path) { return path.trim(); @@ -124,34 +125,59 @@ function renderDetail(key) { return; } + if (detail.items[0].file_previews) { + reviewState = {key, index: 0, choices: new Map()}; + repairPanel.hidden = false; + renderReviewGroup(); + return; + } + drilldownList.innerHTML = detail.items.map((item, index) => ` -
+
${escapeHtml(item.filename || item.path || 'Untitled item')} - ${item.files ? 'Choose this group to review a safe cleanup →' : ''}
`).join(''); } -function chooseDuplicate(detailKey, index) { - const group = latestAnalysis?.details?.[detailKey]?.items?.[index]; - if (!group?.files) return; - selectedDuplicateGroup = group; +function renderReviewGroup() { + const groups = latestAnalysis.details[reviewState.key].items; + const group = groups[reviewState.index]; + const chosen = reviewState.choices.get(reviewState.index); + drilldownCount.textContent = `${reviewState.index + 1} of ${groups.length} · ${reviewState.choices.size} approved`; + drilldownSummary.textContent = 'Listen to each candidate, choose the keeper, and we’ll move to the next group. Skip anything uncertain.'; + drilldownList.innerHTML = ` +
+
Comparing now${escapeHtml(group.filename)}
${reviewState.choices.size} selected
+
${group.file_previews.map((file, index) => ` +
+ Option ${index + 1} + ${escapeHtml(file.path.split('/').pop())} + ${escapeHtml(file.path)} +
+ +
+ `).join('')}
+
+
`; + updateBatchSummary(); +} + +function updateBatchSummary() { + if (!reviewState) return; + const groups = latestAnalysis.details[reviewState.key].items; + repairChoice.innerHTML = `
${reviewState.choices.size} group${reviewState.choices.size === 1 ? '' : 's'} approved${groups.length - reviewState.choices.size} skipped or still awaiting a decision
`; + previewRepairButton.disabled = reviewState.choices.size === 0; previewedRepair = null; applyRepairButton.disabled = true; repairPreview.hidden = true; - repairMessage.textContent = ''; - repairChoice.innerHTML = group.files.map((file, fileIndex) => ` -
- `).join(''); - repairPanel.hidden = false; - repairPanel.scrollIntoView({behavior: 'smooth', block: 'start'}); } function repairPayload() { - const keeper = document.querySelector('input[name="keeper"]:checked')?.value; - if (!selectedDuplicateGroup || !keeper) throw new Error('Choose a file to keep'); - return {serato: expandHome(document.querySelector('#serato-path').value), music: expandHome(document.querySelector('#music-path').value), keeper, group_files: selectedDuplicateGroup.files, backup_limit: document.querySelector('#keep-all-backups').checked ? null : Number(document.querySelector('#backup-limit').value)}; + if (!reviewState?.choices.size) throw new Error('Choose at least one keeper'); + const groups = latestAnalysis.details[reviewState.key].items; + const choices = Array.from(reviewState.choices, ([index, keeper]) => ({keeper, group_files: groups[index].files})); + return {serato: expandHome(document.querySelector('#serato-path').value), music: expandHome(document.querySelector('#music-path').value), choices, backup_limit: document.querySelector('#keep-all-backups').checked ? null : Number(document.querySelector('#backup-limit').value)}; } async function requestRepair(endpoint) { @@ -164,8 +190,8 @@ async function requestRepair(endpoint) { previewRepairButton.addEventListener('click', async () => { repairMessage.textContent = 'Checking the plan…'; applyRepairButton.disabled = true; try { - previewedRepair = await requestRepair('/api/duplicates/preview'); - repairPreview.innerHTML = `Ready to protect and consolidate

${previewedRepair.replaced.length} duplicate file(s) will be backed up, then replaced with shortcuts to the keeper. ${previewedRepair.metadata_backups} Serato metadata file(s) will also be copied into the rollback snapshot. Database V2 will not be changed.

`; + previewedRepair = await requestRepair('/api/duplicates/batch/preview'); + repairPreview.innerHTML = `One safe plan for ${previewedRepair.choice_count} approved group(s)

${previewedRepair.replaced.length} duplicate file(s) will be backed up, then replaced with shortcuts to their selected keepers. ${previewedRepair.metadata_backups} Serato metadata file(s) will also be copied into the rollback snapshot. Database V2 will not be changed.

`; repairPreview.hidden = false; applyRepairButton.disabled = false; repairMessage.textContent = 'Preview complete. Nothing has changed yet.'; } catch (error) { repairMessage.textContent = error.message; } @@ -175,7 +201,7 @@ applyRepairButton.addEventListener('click', async () => { if (!previewedRepair) return; applyRepairButton.disabled = true; repairMessage.textContent = 'Creating the backup before making changes…'; try { - const result = await requestRepair('/api/duplicates/apply'); + const result = await requestRepair('/api/duplicates/batch/apply'); latestBackup = result.backup; repairMessage.textContent = `Cleanup complete. Restore backup: ${result.backup}`; previewRepairButton.disabled = true; @@ -197,7 +223,6 @@ restoreRepairButton.addEventListener('click', async () => { } catch (error) { repairMessage.textContent = error.message; restoreRepairButton.disabled = false; } }); -repairChoice.addEventListener('change', () => { previewedRepair = null; applyRepairButton.disabled = true; repairPreview.hidden = true; repairMessage.textContent = 'Keeper changed. Preview the plan again.'; }); async function handleFileAction(event) { const previewButton = event.target.closest('[data-audio-url]'); const revealButton = event.target.closest('[data-reveal-token]'); @@ -224,8 +249,28 @@ async function handleFileAction(event) { closeAudioPreview.addEventListener('click', () => { audioPlayer.pause(); audioPlayer.removeAttribute('src'); audioPlayer.load(); audioPreview.hidden = true; }); repairChoice.addEventListener('click', handleFileAction); -drilldownList.addEventListener('click', async (event) => { if (await handleFileAction(event)) return; const item = event.target.closest('[data-duplicate-index]'); if (item) chooseDuplicate(document.querySelector('.drill-trigger.selected')?.dataset.detail, Number(item.dataset.duplicateIndex)); }); -drilldownList.addEventListener('keydown', (event) => { if (event.key !== 'Enter' && event.key !== ' ') return; const item = event.target.closest('[data-duplicate-index]'); if (item) { event.preventDefault(); chooseDuplicate(document.querySelector('.drill-trigger.selected')?.dataset.detail, Number(item.dataset.duplicateIndex)); } }); +drilldownList.addEventListener('click', async (event) => { + if (await handleFileAction(event) || !reviewState) return; + const groups = latestAnalysis.details[reviewState.key].items; + const winner = event.target.closest('[data-choose-winner]'); + if (winner) { + const fileIndex = Number(winner.dataset.chooseWinner); + reviewState.choices.set(reviewState.index, groups[reviewState.index].files[fileIndex]); + if (reviewState.index < groups.length - 1) reviewState.index += 1; + renderReviewGroup(); + return; + } + if (event.target.closest('[data-review-skip]')) { + reviewState.choices.delete(reviewState.index); + if (reviewState.index < groups.length - 1) reviewState.index += 1; + renderReviewGroup(); + return; + } + if (event.target.closest('[data-review-previous]') && reviewState.index > 0) { + reviewState.index -= 1; + renderReviewGroup(); + } +}); function render(data) { latestAnalysis = data; diff --git a/serato_doctor/webui/index.html b/serato_doctor/webui/index.html index 2b1ee6a..bc55502 100644 --- a/serato_doctor/webui/index.html +++ b/serato_doctor/webui/index.html @@ -93,8 +93,8 @@ - + diff --git a/serato_doctor/webui/layout-fixes.css b/serato_doctor/webui/layout-fixes.css index 5c67c2f..bf4780a 100644 --- a/serato_doctor/webui/layout-fixes.css +++ b/serato_doctor/webui/layout-fixes.css @@ -8,6 +8,137 @@ } } +.review-workspace { + padding: clamp(16px, 3vw, 24px); + border: 1px solid rgba(155, 135, 245, .18); + border-radius: 16px; + background: rgba(255, 255, 255, .02); +} + +.review-heading, +.review-navigation { + display: flex; + align-items: center; + justify-content: space-between; + gap: 16px; +} + +.review-heading span, +.batch-summary span { + color: var(--muted); + font-size: 10px; +} + +.review-heading strong { + display: block; + margin-top: 5px; + font-size: 14px; +} + +.review-candidates { + display: grid; + grid-template-columns: repeat(2, minmax(0, 1fr)); + gap: 12px; + margin: 18px 0; +} + +.review-candidate { + display: flex; + flex-direction: column; + gap: 9px; + min-width: 0; + padding: 16px; + border: 1px solid var(--line); + border-radius: 14px; + background: rgba(6, 8, 12, .28); +} + +.review-candidate.winner { + border-color: rgba(84, 212, 154, .52); + background: rgba(84, 212, 154, .06); +} + +.candidate-number { + color: var(--violet); + font-size: 9px; + font-weight: 750; + text-transform: uppercase; + letter-spacing: .1em; +} + +.review-candidate > strong, +.review-candidate > small { + overflow-wrap: anywhere; +} + +.review-candidate > strong { + font-size: 12px; +} + +.review-candidate > small { + flex: 1; + color: var(--muted); + font-size: 9px; + line-height: 1.5; +} + +.choose-winner { + margin-top: 4px; + border: 0; + border-radius: 10px; + padding: 10px 12px; + background: linear-gradient(135deg, #907be9, #6e59cf); + color: white; + font: 750 11px/1 inherit; + cursor: pointer; +} + +.winner .choose-winner { + background: rgba(84, 212, 154, .18); + color: #8ce4b8; +} + +.review-navigation button { + border: 1px solid var(--line); + border-radius: 9px; + padding: 8px 11px; + background: rgba(255, 255, 255, .04); + color: var(--muted); + font: 700 10px/1 inherit; + cursor: pointer; +} + +.review-navigation button:disabled { + opacity: .35; +} + +.batch-summary { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + padding: 14px; + border: 1px solid rgba(102, 217, 232, .2); + border-radius: 12px; + background: rgba(102, 217, 232, .05); +} + +.batch-summary strong { + font-size: 12px; +} + +@media (max-width: 760px) { + .review-candidates { + grid-template-columns: 1fr; + } + + .review-heading, + .batch-summary { + align-items: flex-start; + flex-direction: column; + } +} + .file-compare-row { display: grid; gap: 8px; diff --git a/tests/test_repair.py b/tests/test_repair.py index a318876..fb42ce2 100644 --- a/tests/test_repair.py +++ b/tests/test_repair.py @@ -5,6 +5,7 @@ import pytest from serato_doctor.repair import ( BACKUP_FOLDER, apply_duplicate_repair, + apply_duplicate_repair_batch, list_backups, plan_duplicate_repair, restore_backup, @@ -72,3 +73,29 @@ def test_backup_rotation_can_be_limited_or_unlimited(tmp_path): assert len(list(root.iterdir())) == 3 rotate_backups(root, 2) assert {path.name for path in root.iterdir()} == {"002", "003"} + + +def test_batch_repair_uses_one_backup_and_restores_every_group(tmp_path): + serato, first_keeper, first_duplicate = library(tmp_path) + second_keeper = tmp_path / "Music" / "Main" / "Other.mp3" + second_duplicate = tmp_path / "Music" / "Old" / "Other.mp3" + second_keeper.write_bytes(b"other-keeper") + second_duplicate.write_bytes(b"other-duplicate") + plans = ( + plan_duplicate_repair( + first_keeper, (first_keeper, first_duplicate), serato + ), + plan_duplicate_repair( + second_keeper, (second_keeper, second_duplicate), serato + ), + ) + + receipt = apply_duplicate_repair_batch(plans, serato, backup_limit=10) + + assert first_duplicate.is_symlink() + assert second_duplicate.is_symlink() + assert len(list((serato / BACKUP_FOLDER).iterdir())) == 1 + assert len(receipt.replaced) == 2 + restore_backup(receipt.backup) + assert first_duplicate.read_bytes() == b"duplicate" + assert second_duplicate.read_bytes() == b"other-duplicate" diff --git a/tests/test_web.py b/tests/test_web.py index 7a68cf1..87ca5a2 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -9,6 +9,7 @@ from serato_doctor.web import ( _verified_audio, analyze_paths, duplicate_repair, + duplicate_repair_batch, ) @@ -113,3 +114,29 @@ def test_duplicate_details_include_preview_and_finder_controls(tmp_path): assert len(group["file_previews"]) == 2 assert group["file_previews"][0]["audio_url"].startswith("/api/audio?") assert group["file_previews"][0]["reveal_token"] + + +def test_batch_preview_combines_approved_groups_without_changes(tmp_path): + serato = tmp_path / "_Serato_" + serato.mkdir() + music = tmp_path / "Music" + choices = [] + for filename in ("First.mp3", "Second.mp3"): + keeper = music / "A" / filename + duplicate = music / "B" / filename + keeper.parent.mkdir(parents=True, exist_ok=True) + duplicate.parent.mkdir(parents=True, exist_ok=True) + keeper.write_bytes(b"keeper") + duplicate.write_bytes(b"duplicate") + choices.append( + {"keeper": str(keeper), "group_files": [str(keeper), str(duplicate)]} + ) + + result = duplicate_repair_batch( + serato, music, choices, backup_limit=10 + ) + + assert result["applied"] is False + assert result["choice_count"] == 2 + assert len(result["replaced"]) == 2 + assert all(not Path(choice["group_files"][1]).is_symlink() for choice in choices)