From 9f1ce946904e81d04fc916c33023b3f312c8558e Mon Sep 17 00:00:00 2001 From: Francwa Date: Thu, 21 May 2026 07:46:13 +0200 Subject: [PATCH] refactor(application): inject kb/prober into resolve_destination use cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the module-level _KB / _PROBER singletons from alfred/application/filesystem/resolve_destination.py. The four resolve_{season,episode,movie,series}_destination use cases now take kb: ReleaseKnowledge and prober: MediaProber as required arguments, matching the shape of inspect_release. The singletons now live at the agent-tools frontier (alfred/agent/tools/filesystem.py), where the LLM-facing wrappers instantiate YamlReleaseKnowledge / FfprobeMediaProber once and thread them through. The wrappers' Python signatures are unchanged — the inspect-based JSON-schema generator in agent/registry.py still sees the same LLM-passable params. analyze_release drops the dirty 'from ... import _KB' indirection. Tests inject their own stubs by keyword (prober=_StubProber(...)) via thin convenience wrappers, replacing the prior monkeypatch.setattr(rd, '_PROBER', ...) pattern. testing/debug_release.py: instantiate YamlReleaseKnowledge() / FfprobeMediaProber() inline at the two call sites. Suite: 1077 passed. --- CHANGELOG.md | 15 ++++ alfred/agent/tools/filesystem.py | 26 +++++- .../filesystem/resolve_destination.py | 54 ++++++------ testing/debug_release.py | 20 ++++- tests/application/test_resolve_destination.py | 82 ++++++++++++++----- 5 files changed, 144 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ace31ce..1bce98a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,21 @@ callers). ### Changed +- **`resolve_destination` use cases take `kb` / `prober` as required + params; module-level singletons gone.** The four + `resolve_{season,episode,movie,series}_destination` use cases now + accept `kb: ReleaseKnowledge` and `prober: MediaProber` as required + arguments, matching the shape of `inspect_release`. The module-level + `_KB = YamlReleaseKnowledge()` and `_PROBER = FfprobeMediaProber()` + singletons that previously lived in + `alfred/application/filesystem/resolve_destination.py` are removed — + the application layer no longer reaches into infrastructure. The + singletons now live at the agent-tools frontier + (`alfred/agent/tools/filesystem.py`), where the LLM-facing wrappers + instantiate them once and thread them through. `analyze_release` no + longer needs the dirty `from ... import _KB` indirection. Tests + inject their own stubs by keyword (`prober=_StubProber(...)`) instead + of monkeypatching a module attribute. - **`ParsePath` enum renamed to `TokenizationRoute`.** The old name collided with `pathlib.Path` in code-reading mental models, and was one letter from `parse_path` (the field that holds the value) — making diff --git a/alfred/agent/tools/filesystem.py b/alfred/agent/tools/filesystem.py index 5eb29a2..387a8f7 100644 --- a/alfred/agent/tools/filesystem.py +++ b/alfred/agent/tools/filesystem.py @@ -26,10 +26,15 @@ from alfred.application.filesystem.resolve_destination import ( resolve_series_destination as _resolve_series_destination, ) from alfred.infrastructure.filesystem import FileManager, create_folder, move +from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge from alfred.infrastructure.metadata import MetadataStore from alfred.infrastructure.persistence import get_memory from alfred.infrastructure.probe import FfprobeMediaProber +# Agent-tools frontier: this is the legitimate home for the singletons that +# back every LLM-exposed wrapper. The use cases below take ``kb`` / ``prober`` +# as required params; tests inject their own stubs. +_KB = YamlReleaseKnowledge() _PROBER = FfprobeMediaProber() _LEARNED_ROOT = Path(_alfred_pkg.__file__).parent.parent / "data" / "knowledge" @@ -60,7 +65,13 @@ def resolve_season_destination( ) -> dict[str, Any]: """Thin tool wrapper — semantics live in alfred/agent/tools/specs/resolve_season_destination.yaml.""" return _resolve_season_destination( - release_name, tmdb_title, tmdb_year, confirmed_folder, source_path + release_name, + tmdb_title, + tmdb_year, + _KB, + _PROBER, + confirmed_folder, + source_path, ).to_dict() @@ -78,6 +89,8 @@ def resolve_episode_destination( source_file, tmdb_title, tmdb_year, + _KB, + _PROBER, tmdb_episode_title, confirmed_folder, ).to_dict() @@ -91,7 +104,7 @@ def resolve_movie_destination( ) -> dict[str, Any]: """Thin tool wrapper — semantics live in alfred/agent/tools/specs/resolve_movie_destination.yaml.""" return _resolve_movie_destination( - release_name, source_file, tmdb_title, tmdb_year + release_name, source_file, tmdb_title, tmdb_year, _KB, _PROBER ).to_dict() @@ -104,7 +117,13 @@ def resolve_series_destination( ) -> dict[str, Any]: """Thin tool wrapper — semantics live in alfred/agent/tools/specs/resolve_series_destination.yaml.""" return _resolve_series_destination( - release_name, tmdb_title, tmdb_year, confirmed_folder, source_path + release_name, + tmdb_title, + tmdb_year, + _KB, + _PROBER, + confirmed_folder, + source_path, ).to_dict() @@ -191,7 +210,6 @@ def set_path_for_folder(folder_name: str, path_value: str) -> dict[str, Any]: def analyze_release(release_name: str, source_path: str) -> dict[str, Any]: """Thin tool wrapper — semantics live in alfred/agent/tools/specs/analyze_release.yaml.""" - from alfred.application.filesystem.resolve_destination import _KB # noqa: PLC0415 from alfred.application.release import inspect_release # noqa: PLC0415 result = inspect_release(release_name, Path(source_path), _KB, _PROBER) diff --git a/alfred/application/filesystem/resolve_destination.py b/alfred/application/filesystem/resolve_destination.py index 437b768..b823b26 100644 --- a/alfred/application/filesystem/resolve_destination.py +++ b/alfred/application/filesystem/resolve_destination.py @@ -26,34 +26,30 @@ from alfred.application.release import inspect_release from alfred.domain.release import parse_release from alfred.domain.release.ports import ReleaseKnowledge from alfred.domain.release.value_objects import ParsedRelease -from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge +from alfred.domain.shared.ports import MediaProber from alfred.infrastructure.persistence import get_memory -from alfred.infrastructure.probe import FfprobeMediaProber logger = logging.getLogger(__name__) -# Single module-level knowledge instance. YAML is loaded once at first import. -# Tests that need a custom KB can monkeypatch this attribute. -_KB: ReleaseKnowledge = YamlReleaseKnowledge() -# Module-level prober — same singleton style as _KB. Tests that need a custom -# adapter can monkeypatch this attribute. -_PROBER = FfprobeMediaProber() - - -def _resolve_parsed(release_name: str, source_path: str | None) -> ParsedRelease: +def _resolve_parsed( + release_name: str, + source_path: str | None, + kb: ReleaseKnowledge, + prober: MediaProber, +) -> ParsedRelease: """Pick the right entry point depending on whether we have a path. When ``source_path`` is provided and points to something that exists, - we run the full inspection pipeline so probe data can refresh - ``tech_string`` (which feeds every filename builder). Otherwise we - fall back to a parse-only path — same behavior as before. + we run the full inspection pipeline so probe data can refresh tech + fields (which feed every filename builder). Otherwise we fall back + to a parse-only path — same behavior as before. """ if source_path: path = Path(source_path) if path.exists(): - return inspect_release(release_name, path, _KB, _PROBER).parsed - parsed, _ = parse_release(release_name, _KB) + return inspect_release(release_name, path, kb, prober).parsed + parsed, _ = parse_release(release_name, kb) return parsed @@ -259,6 +255,8 @@ def resolve_season_destination( release_name: str, tmdb_title: str, tmdb_year: int, + kb: ReleaseKnowledge, + prober: MediaProber, confirmed_folder: str | None = None, source_path: str | None = None, ) -> ResolvedSeasonDestination: @@ -280,8 +278,8 @@ def resolve_season_destination( message="TV show library path is not configured.", ) - parsed = _resolve_parsed(release_name, source_path) - tmdb_title_safe = _KB.sanitize_for_fs(tmdb_title) + parsed = _resolve_parsed(release_name, source_path, kb, prober) + tmdb_title_safe = kb.sanitize_for_fs(tmdb_title) computed_name = parsed.show_folder_name(tmdb_title_safe, tmdb_year) resolved = _resolve_series_folder( @@ -314,6 +312,8 @@ def resolve_episode_destination( source_file: str, tmdb_title: str, tmdb_year: int, + kb: ReleaseKnowledge, + prober: MediaProber, tmdb_episode_title: str | None = None, confirmed_folder: str | None = None, ) -> ResolvedEpisodeDestination: @@ -332,11 +332,11 @@ def resolve_episode_destination( message="TV show library path is not configured.", ) - parsed = _resolve_parsed(release_name, source_file) + parsed = _resolve_parsed(release_name, source_file, kb, prober) ext = Path(source_file).suffix - tmdb_title_safe = _KB.sanitize_for_fs(tmdb_title) + tmdb_title_safe = kb.sanitize_for_fs(tmdb_title) tmdb_episode_title_safe = ( - _KB.sanitize_for_fs(tmdb_episode_title) if tmdb_episode_title else None + kb.sanitize_for_fs(tmdb_episode_title) if tmdb_episode_title else None ) computed_name = parsed.show_folder_name(tmdb_title_safe, tmdb_year) @@ -375,6 +375,8 @@ def resolve_movie_destination( source_file: str, tmdb_title: str, tmdb_year: int, + kb: ReleaseKnowledge, + prober: MediaProber, ) -> ResolvedMovieDestination: """ Compute destination paths for a movie file. @@ -392,9 +394,9 @@ def resolve_movie_destination( message="Movie library path is not configured.", ) - parsed = _resolve_parsed(release_name, source_file) + parsed = _resolve_parsed(release_name, source_file, kb, prober) ext = Path(source_file).suffix - tmdb_title_safe = _KB.sanitize_for_fs(tmdb_title) + tmdb_title_safe = kb.sanitize_for_fs(tmdb_title) folder_name = parsed.movie_folder_name(tmdb_title_safe, tmdb_year) filename = parsed.movie_filename(tmdb_title_safe, tmdb_year, ext) @@ -416,6 +418,8 @@ def resolve_series_destination( release_name: str, tmdb_title: str, tmdb_year: int, + kb: ReleaseKnowledge, + prober: MediaProber, confirmed_folder: str | None = None, source_path: str | None = None, ) -> ResolvedSeriesDestination: @@ -435,8 +439,8 @@ def resolve_series_destination( message="TV show library path is not configured.", ) - parsed = _resolve_parsed(release_name, source_path) - tmdb_title_safe = _KB.sanitize_for_fs(tmdb_title) + parsed = _resolve_parsed(release_name, source_path, kb, prober) + tmdb_title_safe = kb.sanitize_for_fs(tmdb_title) computed_name = parsed.show_folder_name(tmdb_title_safe, tmdb_year) resolved = _resolve_series_folder( diff --git a/testing/debug_release.py b/testing/debug_release.py index e639b4a..d92e14f 100644 --- a/testing/debug_release.py +++ b/testing/debug_release.py @@ -124,8 +124,16 @@ def dry_run(release_name: str) -> None: from alfred.application.filesystem.resolve_destination import ( resolve_season_destination, ) + from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge + from alfred.infrastructure.probe import FfprobeMediaProber - result = resolve_season_destination(release_name, tmdb_title, tmdb_year) + result = resolve_season_destination( + release_name, + tmdb_title, + tmdb_year, + YamlReleaseKnowledge(), + FfprobeMediaProber(), + ) d = result.to_dict() print() print(json.dumps(d, indent=2, ensure_ascii=False)) @@ -203,8 +211,16 @@ def do_move(release_name: str, source_folder: str | None = None) -> None: from alfred.application.filesystem.resolve_destination import ( resolve_season_destination, ) + from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge + from alfred.infrastructure.probe import FfprobeMediaProber - result = resolve_season_destination(release_name, tmdb_title, tmdb_year) + result = resolve_season_destination( + release_name, + tmdb_title, + tmdb_year, + YamlReleaseKnowledge(), + FfprobeMediaProber(), + ) d = result.to_dict() if d["status"] == "needs_clarification": diff --git a/tests/application/test_resolve_destination.py b/tests/application/test_resolve_destination.py index 8cd57fa..4cac796 100644 --- a/tests/application/test_resolve_destination.py +++ b/tests/application/test_resolve_destination.py @@ -31,13 +31,53 @@ from alfred.application.filesystem.resolve_destination import ( _Clarification, _find_existing_tvshow_folders, _resolve_series_folder, - resolve_episode_destination, - resolve_movie_destination, - resolve_season_destination, - resolve_series_destination, + resolve_episode_destination as _resolve_episode_destination, + resolve_movie_destination as _resolve_movie_destination, + resolve_season_destination as _resolve_season_destination, + resolve_series_destination as _resolve_series_destination, ) +from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge from alfred.infrastructure.persistence import Memory, set_memory +_KB = YamlReleaseKnowledge() + + +class _NullProber: + """Default prober stub — never returns probe data.""" + + def list_subtitle_streams(self, video): # pragma: no cover + return [] + + def probe(self, video): + return None + + +_DEFAULT_PROBER = _NullProber() + + +def resolve_season_destination(*args, prober=None, **kwargs): + return _resolve_season_destination( + *args, kb=_KB, prober=prober or _DEFAULT_PROBER, **kwargs + ) + + +def resolve_episode_destination(*args, prober=None, **kwargs): + return _resolve_episode_destination( + *args, kb=_KB, prober=prober or _DEFAULT_PROBER, **kwargs + ) + + +def resolve_movie_destination(*args, prober=None, **kwargs): + return _resolve_movie_destination( + *args, kb=_KB, prober=prober or _DEFAULT_PROBER, **kwargs + ) + + +def resolve_series_destination(*args, prober=None, **kwargs): + return _resolve_series_destination( + *args, kb=_KB, prober=prober or _DEFAULT_PROBER, **kwargs + ) + REL_EPISODE = "Oz.S01E01.1080p.WEBRip.x265-KONTRAST" REL_SEASON = "Oz.S03.1080p.WEBRip.x265-KONTRAST" REL_MOVIE = "Inception.2010.1080p.BluRay.x265-GROUP" @@ -365,46 +405,40 @@ class TestProbeEnrichmentWiring: should pick up ffprobe data via inspect_release and let the enriched tech_string land in the destination name.""" - def test_movie_picks_up_probe_quality( - self, cfg_memory, tmp_path, monkeypatch - ): - from alfred.application.filesystem import resolve_destination as rd - - monkeypatch.setattr(rd, "_PROBER", _StubProber(_stereo_movie_info())) + def test_movie_picks_up_probe_quality(self, cfg_memory, tmp_path): # Release name parses to "movie" but is missing the quality token; # probe must supply 1080p and refresh tech_string. bare_name = "Inception.2010.BluRay.x264-GROUP" video = tmp_path / "movie.mkv" video.write_bytes(b"") - out = resolve_movie_destination(bare_name, str(video), "Inception", 2010) + out = resolve_movie_destination( + bare_name, + str(video), + "Inception", + 2010, + prober=_StubProber(_stereo_movie_info()), + ) assert out.status == "ok" # tech_string -> "1080p.BluRay.x264" -> "1080p" shows up in names. assert "1080p" in out.movie_folder_name assert "1080p" in out.filename - def test_movie_skips_probe_when_path_missing(self, cfg_memory, monkeypatch): + def test_movie_skips_probe_when_path_missing(self, cfg_memory): # If the file doesn't exist, no probe runs (the stub would have # injected 1080p — its absence proves the skip). - from alfred.application.filesystem import resolve_destination as rd - - monkeypatch.setattr(rd, "_PROBER", _StubProber(_stereo_movie_info())) out = resolve_movie_destination( "Inception.2010.BluRay.x264-GROUP", "/nowhere/m.mkv", "Inception", 2010, + prober=_StubProber(_stereo_movie_info()), ) assert out.status == "ok" assert "1080p" not in out.movie_folder_name - def test_season_picks_up_probe_via_source_path( - self, cfg_memory, tmp_path, monkeypatch - ): - from alfred.application.filesystem import resolve_destination as rd - - monkeypatch.setattr(rd, "_PROBER", _StubProber(_stereo_movie_info())) + def test_season_picks_up_probe_via_source_path(self, cfg_memory, tmp_path): # Season pack name missing quality token; probe must add it. bare_name = "Oz.S03.BluRay.x265-KONTRAST" release_dir = tmp_path / bare_name @@ -412,7 +446,11 @@ class TestProbeEnrichmentWiring: (release_dir / "episode.mkv").write_bytes(b"") out = resolve_season_destination( - bare_name, "Oz", 1997, source_path=str(release_dir) + bare_name, + "Oz", + 1997, + source_path=str(release_dir), + prober=_StubProber(_stereo_movie_info()), ) assert out.status == "ok"