From 157d19f3b9253206fe6ec369bf39a17da61cf8c4 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 7 Jun 2026 00:58:09 -0700 Subject: [PATCH] Post-merge #801 follow-ups: un-silence the retry engine's logs + register origin-history.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings from PR #801, fixed as promised after merge: - core/imports/version_mismatch_fallback.py and core/downloads/task_worker.py used bare getLogger(__name__) — outside the soulsync.* namespace where handlers attach, so the entire retry story (the [Modal Worker] search/retry walk and, critically, the "accepting best quarantined candidate as last resort" warning) never reached app.log. Same bug class as the prepare.py fix; both moved to get_logger. A repo sweep shows 61 more modules with the same pattern — noted as its own cleanup project. - the full-suite run also caught a miss of MINE, not the PR's: the new origin-history.js wasn't registered in the script-split integrity test, so openDownloadOriginsModal failed onclick coverage. Registered — and the onclick scan now iterates the NON_SPLIT_JS registry instead of its own hardcoded copy, so the next standalone module can't silently skip coverage. Merged dev verified: PR's 77 tests + 4233 full-suite tests pass (the only exclusion is the eternal soundcloud /app file); integrity suite 64/64. --- core/downloads/task_worker.py | 8 ++++++-- core/imports/version_mismatch_fallback.py | 9 +++++++-- tests/test_script_split_integrity.py | 8 +++++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/core/downloads/task_worker.py b/core/downloads/task_worker.py index 1e27cbc4..71e49933 100644 --- a/core/downloads/task_worker.py +++ b/core/downloads/task_worker.py @@ -19,7 +19,6 @@ a large web_server.py helper that will get its own lift in subsequent PRs. from __future__ import annotations -import logging import re import traceback from dataclasses import dataclass @@ -27,8 +26,13 @@ from typing import Any, Callable, Optional from core.runtime_state import download_batches, download_tasks, tasks_lock from core.spotify_client import Track as SpotifyTrack +from utils.logging_config import get_logger -logger = logging.getLogger(__name__) +# Must live under the soulsync.* namespace — handlers only attach there. The +# old bare getLogger(__name__) ("core.downloads.task_worker") had no handler, +# so the entire [Modal Worker] story — search queries, retry walks, candidate +# decisions — never reached app.log. +logger = get_logger("downloads.task_worker") def _resolve_worker_source(username): diff --git a/core/imports/version_mismatch_fallback.py b/core/imports/version_mismatch_fallback.py index 7eaab479..3e0ef92d 100644 --- a/core/imports/version_mismatch_fallback.py +++ b/core/imports/version_mismatch_fallback.py @@ -26,11 +26,16 @@ quarantine store + re-import dispatch via injected callables. from __future__ import annotations -import logging import re from typing import Any, Callable, Dict, List, Optional -logger = logging.getLogger(__name__) +from utils.logging_config import get_logger + +# Must live under the soulsync.* namespace — handlers only attach there, so a +# bare getLogger(__name__) sent every line (including the critical "accepting +# best quarantined candidate as last resort" warning) into the void instead of +# app.log. Same bug class as the prepare.py fix. +logger = get_logger("imports.version_fallback") # Matches the reason string written by acoustid_verification's version gate: # "Version mismatch: expected '' (<exp>) but file is '<title>' (<got>)" diff --git a/tests/test_script_split_integrity.py b/tests/test_script_split_integrity.py index c1a628e1..876f4d58 100644 --- a/tests/test_script_split_integrity.py +++ b/tests/test_script_split_integrity.py @@ -52,7 +52,7 @@ SPLIT_MODULES = [ # Other JS files that exist in static/ but are NOT part of the split NON_SPLIT_JS = {"setup-wizard.js", "docs.js", "helper.js", "particles.js", "worker-orbs.js", - "enrichment-manager.js"} + "enrichment-manager.js", "origin-history.js"} # Pre-existing duplicate helper functions that lived in the original monolith. # In a plain <script> context the last-loaded declaration wins. These are NOT @@ -216,8 +216,10 @@ class TestOnclickCoverage: text = _read(_STATIC / module) self.all_fns.update(_all_function_decls(text)) - # Also include non-split JS files that are loaded - for extra in ("setup-wizard.js", "docs.js", "helper.js", "enrichment-manager.js"): + # Also include non-split JS files that are loaded — driven by the + # NON_SPLIT_JS registry so a newly added standalone module can't be + # silently missing from onclick coverage (origin-history.js was). + for extra in sorted(NON_SPLIT_JS): path = _STATIC / extra if path.exists(): self.all_fns.update(_all_function_decls(_read(path)))