From 4a1b3d0627b58b78746b06b72796fd62d87abee6 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 7 Jun 2026 11:04:28 -0700 Subject: [PATCH] PR #801 follow-up: default-config template contradicted the retry engine's documented default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed all 7 requeue tests that passed locally. Root cause is a real shipping bug, not test flake: config/settings.py's default template set retry_next_candidate_on_mismatch: False ("Default off — opt-in") while the monitor reads it with inline default True and the PR documents it as ON. Outcome split the userbase: a FRESH install (or CI's clean runner) gets the template key = retry engine silently OFF; an existing config.json lacks the key = inline True wins = engine ON. Same code, opposite behavior, decided by install age. - template aligned to True (the documented + approved default; existing installs already behave this way via the inline default) - the requeue tests now pin the toggle ON via the wiring helper instead of reading the runner's ambient config — CI's fresh defaults vs a dev's lived-in config.json must never decide whether they pass. _patch_config composes (it wraps the pinned get and falls through). 64 retry-engine tests pass; fresh-default simulation confirms the toggle resolves True. --- config/settings.py | 8 ++++++-- tests/imports/test_import_pipeline.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/config/settings.py b/config/settings.py index f729a2a4..a5cb62de 100644 --- a/config/settings.py +++ b/config/settings.py @@ -497,8 +497,12 @@ class ConfigManager: "post_processing": { # When a download is quarantined (AcoustID mismatch, integrity / # duration failure), retry the next-best candidate instead of - # failing outright. Default off — opt-in. - "retry_next_candidate_on_mismatch": False, + # failing outright. Default ON (PR #801's documented default — + # the monitor reads this with inline default True; this template + # said False, so fresh installs silently shipped with the retry + # engine off while existing configs got it on. CI caught the + # split: its fresh default config failed all 7 requeue tests). + "retry_next_candidate_on_mismatch": True, # Opt-in exhaustive retry: budget retries PER SOURCE so every # source (Soulseek, then HiFi/Tidal/…) gets its own attempts # before the track gives up. Default off (single global cap). diff --git a/tests/imports/test_import_pipeline.py b/tests/imports/test_import_pipeline.py index e602ab81..a8b550aa 100644 --- a/tests/imports/test_import_pipeline.py +++ b/tests/imports/test_import_pipeline.py @@ -326,6 +326,18 @@ def _wire_retry_engine(monkeypatch): monkeypatch.setattr(monitor, "missing_download_executor", _Exec()) monkeypatch.setattr(monitor, "_download_track_worker", lambda task_id, batch_id: None) monkeypatch.setattr(monitor, "MAX_QUARANTINE_RETRIES", 5) + + # Pin the retry toggle ON instead of reading the runner's ambient config — + # CI's fresh default config vs a dev's lived-in config.json must not + # decide whether these tests pass (they did: 7 failures, CI-only). + real_get = monitor.config_manager.get + + def _pinned_get(key, default=None): + if key == "post_processing.retry_next_candidate_on_mismatch": + return True + return real_get(key, default) + + monkeypatch.setattr(monitor.config_manager, "get", _pinned_get) return submitted