From 4fde93613f8ff23be2b48a95a7001d6ac5dd6003 Mon Sep 17 00:00:00 2001 From: Philip Guzman Date: Tue, 30 Jun 2026 17:36:58 -0700 Subject: [PATCH] Add opt-in diagnostic logging --- ROADMAP.md | 1 + docs/design/logging.md | 25 +++++++++++++++++++++++++ serato_doctor/cli.py | 15 +++++++++++++++ serato_doctor/config.py | 16 ++++++++++++++-- serato_doctor/logging.py | 40 ++++++++++++++++++++++++++++++++++++++++ tests/test_cli.py | 4 +++- tests/test_logging.py | 34 ++++++++++++++++++++++++++++++++++ 7 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 docs/design/logging.md create mode 100644 serato_doctor/logging.py create mode 100644 tests/test_logging.py diff --git a/ROADMAP.md b/ROADMAP.md index 8a1eff0..d931f38 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -12,6 +12,7 @@ - [x] Sample library fixtures - [ ] Database V2 read-only parser - [x] Configuration +- [x] Logging ## v0.2 — Diagnostics diff --git a/docs/design/logging.md b/docs/design/logging.md new file mode 100644 index 0000000..4b62f4d --- /dev/null +++ b/docs/design/logging.md @@ -0,0 +1,25 @@ +# Application Logging + +## Problem + +The CLI reports final counts but provides no diagnostic trail when a scan behaves +unexpectedly. Troubleshooting should not require adding print statements or expose +library contents by default. + +## Architecture + +The project uses an isolated standard-library logger. It has no visible output by +default. `--verbose` writes progress to standard error, while `--log-file PATH` +writes an informational audit trail. Normal result lines remain on standard output. + +## Edge Cases + +- Reconfiguring logging in the same process must not duplicate handlers. +- Console and file logging may be enabled together. +- Log messages contain aggregate counts, not track names or crate contents. +- A default scan must remain quiet except for its established result output. + +## Verification + +Tests verify quiet defaults, file output, handler replacement, and unchanged CLI +result lines. The full sample-library scan remains read-only. diff --git a/serato_doctor/cli.py b/serato_doctor/cli.py index fdc3992..3c565c6 100644 --- a/serato_doctor/cli.py +++ b/serato_doctor/cli.py @@ -3,6 +3,7 @@ import argparse from serato_doctor.config import ScanConfig from serato_doctor.crate_parser import parse_crates +from serato_doctor.logging import configure_logging from serato_doctor.models.library import Library from serato_doctor.scanner import scan_audio from serato_doctor.report import write_csv, write_missing_report @@ -20,6 +21,10 @@ def main(): default=[], help="Old library root stored in crates; may be supplied more than once", ) + parser.add_argument( + "--verbose", action="store_true", help="Write diagnostic progress to stderr" + ) + parser.add_argument("--log-file", help="Write scan progress to a log file") args = parser.parse_args() config = ScanConfig.build( @@ -28,7 +33,13 @@ def main(): out=Path(args.out), report=Path(args.report), reference_roots=(Path(root) for root in args.reference_root), + verbose=args.verbose, + log_file=Path(args.log_file) if args.log_file else None, ) + logger = configure_logging(config.verbose, config.log_file) + logger.info("Starting read-only library scan") + logger.debug("Serato directory: %s", config.serato) + logger.debug("Music directory: %s", config.music) library = Library.build( references=parse_crates( @@ -38,9 +49,13 @@ def main(): ) results = library.reconcile_by_filename() missing_count = sum(1 for result in results if not result.exists_by_filename) + logger.info("Parsed %d crate references", len(library.references)) + logger.info("Scanned %d disk tracks", len(library.tracks)) + logger.info("Found %d references missing by filename", missing_count) write_csv(results, config.out) write_missing_report(results, config.report) + logger.info("Wrote CSV and missing-reference reports") print(f"Crate references: {len(library.references)}") print(f"Disk tracks: {len(library.tracks)}") diff --git a/serato_doctor/config.py b/serato_doctor/config.py index 95d4bd9..6ce271a 100644 --- a/serato_doctor/config.py +++ b/serato_doctor/config.py @@ -1,6 +1,6 @@ from dataclasses import dataclass from pathlib import Path -from typing import Iterable, Tuple +from typing import Iterable, Optional, Tuple @dataclass(frozen=True) @@ -12,6 +12,8 @@ class ScanConfig: out: Path report: Path reference_roots: Tuple[Path, ...] = () + verbose: bool = False + log_file: Optional[Path] = None @classmethod def build( @@ -21,5 +23,15 @@ class ScanConfig: out: Path, report: Path, reference_roots: Iterable[Path] = (), + verbose: bool = False, + log_file: Optional[Path] = None, ) -> "ScanConfig": - return cls(serato, music, out, report, tuple(reference_roots)) + return cls( + serato, + music, + out, + report, + tuple(reference_roots), + verbose, + log_file, + ) diff --git a/serato_doctor/logging.py b/serato_doctor/logging.py new file mode 100644 index 0000000..88d3af9 --- /dev/null +++ b/serato_doctor/logging.py @@ -0,0 +1,40 @@ +import logging +from pathlib import Path +from typing import Optional + + +LOGGER_NAME = "serato_doctor" +LOG_FORMAT = "%(asctime)s %(levelname)s %(message)s" + + +def configure_logging( + verbose: bool = False, log_file: Optional[Path] = None +) -> logging.Logger: + """Configure isolated application logging and return the project logger.""" + + logger = logging.getLogger(LOGGER_NAME) + logger.setLevel(logging.DEBUG) + logger.propagate = False + + for handler in logger.handlers[:]: + handler.close() + logger.removeHandler(handler) + + formatter = logging.Formatter(LOG_FORMAT) + + if verbose: + console = logging.StreamHandler() + console.setLevel(logging.DEBUG) + console.setFormatter(formatter) + logger.addHandler(console) + + if log_file is not None: + file_handler = logging.FileHandler(log_file, encoding="utf-8") + file_handler.setLevel(logging.INFO) + file_handler.setFormatter(formatter) + logger.addHandler(file_handler) + + if not logger.handlers: + logger.addHandler(logging.NullHandler()) + + return logger diff --git a/tests/test_cli.py b/tests/test_cli.py index 5fb5128..09878c6 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -37,7 +37,9 @@ def test_cli_writes_reports_and_prints_counts(tmp_path, monkeypatch, capsys): main() - output = capsys.readouterr().out + captured = capsys.readouterr() + output = captured.out + assert captured.err == "" assert "Crate references: 2" in output assert "Disk tracks: 1" in output assert "Missing by filename: 1" in output diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 0000000..ce0fce2 --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,34 @@ +import logging + +from serato_doctor.logging import LOGGER_NAME, configure_logging + + +def test_logging_is_quiet_by_default(capsys): + logger = configure_logging() + + logger.info("not visible") + + assert capsys.readouterr().err == "" + + +def test_logging_writes_aggregate_progress_to_file(tmp_path): + log_path = tmp_path / "scan.log" + logger = configure_logging(log_file=log_path) + + logger.info("Scanned %d disk tracks", 10) + + contents = log_path.read_text(encoding="utf-8") + assert "INFO Scanned 10 disk tracks" in contents + + +def test_reconfiguring_logging_replaces_handlers(): + configure_logging(verbose=True) + logger = configure_logging(verbose=True) + + active_handlers = [ + handler + for handler in logger.handlers + if not isinstance(handler, logging.NullHandler) + ] + assert logger.name == LOGGER_NAME + assert len(active_handlers) == 1