diff --git a/CHANGELOG.md b/CHANGELOG.md index 0405a7a..e3e84b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,17 @@ callers). ### Added +- **`InspectedResult.recommended_action` property** — derived hint that + collapses the orchestrator's go / wait / skip decision into a single + value (``"process"`` / ``"ask_user"`` / ``"skip"``). Centralizes the + exclusion logic that was previously dispersed across road / + media_type / main_video checks at each call site. Ordering is part of + the contract: ``skip`` (no main video, or media_type == ``"other"``) + wins over ``ask_user`` (media_type == ``"unknown"`` or road == + ``"path_of_pain"``) which wins over ``process``. Surfaced through the + ``analyze_release`` tool so the LLM can route on it directly. + 6 new tests in ``tests/application/test_inspect.py`` cover the four + branches and the precedence rules. - **`LanguageRepository` port** in `alfred.domain.shared.ports`. Structural Protocol covering `from_iso`, `from_any`, `all`, `__contains__`, `__len__` — the surface previously coupled to the concrete `LanguageRegistry`. diff --git a/alfred/agent/tools/filesystem.py b/alfred/agent/tools/filesystem.py index 387a8f7..746c913 100644 --- a/alfred/agent/tools/filesystem.py +++ b/alfred/agent/tools/filesystem.py @@ -238,6 +238,7 @@ def analyze_release(release_name: str, source_path: str) -> dict[str, Any]: "probe_used": result.probe_used, "confidence": result.report.confidence, "road": result.report.road, + "recommended_action": result.recommended_action, } diff --git a/alfred/agent/tools/specs/analyze_release.yaml b/alfred/agent/tools/specs/analyze_release.yaml index 17d4ba2..6c6a0d5 100644 --- a/alfred/agent/tools/specs/analyze_release.yaml +++ b/alfred/agent/tools/specs/analyze_release.yaml @@ -82,3 +82,4 @@ returns: probe_used: True when ffprobe successfully enriched the result. confidence: Parser confidence score, 0–100 (higher = more reliable). road: "Parser road: 'easy' (group schema matched), 'shitty' (heuristic but acceptable), or 'path_of_pain' (low confidence — ask the user before auto-routing)." + recommended_action: "Orchestrator hint: 'process' (go straight to resolve_*_destination), 'ask_user' (media_type unknown or road=path_of_pain — confirm with the user first), or 'skip' (no main video, or media_type=other — nothing to organize)." diff --git a/alfred/application/release/inspect.py b/alfred/application/release/inspect.py index ea33b4d..1cc8828 100644 --- a/alfred/application/release/inspect.py +++ b/alfred/application/release/inspect.py @@ -62,6 +62,21 @@ from alfred.domain.shared.media import MediaInfo from alfred.domain.shared.ports import MediaProber +# Media types for which a probe carries no useful information. +_NON_PROBABLE_MEDIA_TYPES = frozenset({"unknown", "other"}) + +# Media types for which there's nothing for the organizer to do. +# ``other`` covers things like games / ISOs / archives sitting on the +# downloads folder. ``unknown`` does NOT belong here — those need a +# user decision, not a skip. +_SKIPPABLE_MEDIA_TYPES = frozenset({"other"}) + +# Roads that signal the parser couldn't reach a confident answer on its +# own. ``Road`` values are kept as strings on the report to avoid a +# cross-package import here. +_ASK_USER_ROADS = frozenset({"path_of_pain"}) + + @dataclass(frozen=True) class InspectedResult: """The full picture of a release: parsed name + filesystem reality. @@ -85,6 +100,10 @@ class InspectedResult: - ``probe_used`` — ``True`` iff ``media_info`` is non-``None`` and ``enrich_from_probe`` actually ran. Explicit flag so callers don't have to re-derive the condition. + - ``recommended_action`` — derived hint for the orchestrator (see + property docstring). Encodes the exclusion / clarification / + go-ahead decision in one place so downstream callers don't + re-implement the same checks. """ parsed: ParsedRelease @@ -94,9 +113,36 @@ class InspectedResult: media_info: MediaInfo | None probe_used: bool + @property + def recommended_action(self) -> str: + """Return one of ``"skip"`` / ``"ask_user"`` / ``"process"``. -# Media types for which a probe carries no useful information. -_NON_PROBABLE_MEDIA_TYPES = frozenset({"unknown", "other"}) + - ``"skip"`` — nothing to organize: + * the source has no main video file, **or** + * ``media_type`` is ``"other"`` (games / ISOs / archives). + - ``"ask_user"`` — a decision is required before any action: + * ``media_type`` is ``"unknown"`` (parser couldn't classify), **or** + * the parse landed on ``Road.PATH_OF_PAIN`` + (low-confidence, malformed name, etc.). + - ``"process"`` — everything else: a confident parse with a + usable media type and a main video on disk. The orchestrator + can move straight to the planning step. + + The check ordering matters: ``"skip"`` wins over ``"ask_user"`` + because if there's no video to organize, no question to the + user can change that. ``"ask_user"`` then wins over + ``"process"`` because a confident parse alone isn't enough if + the type or road still flag uncertainty. + """ + if self.main_video is None: + return "skip" + if self.parsed.media_type.value in _SKIPPABLE_MEDIA_TYPES: + return "skip" + if self.parsed.media_type.value == "unknown": + return "ask_user" + if self.report.road in _ASK_USER_ROADS: + return "ask_user" + return "process" def inspect_release( diff --git a/tests/application/test_inspect.py b/tests/application/test_inspect.py index f095a1f..31f2a21 100644 --- a/tests/application/test_inspect.py +++ b/tests/application/test_inspect.py @@ -263,3 +263,94 @@ class TestFrozen: pass else: # pragma: no cover raise AssertionError("InspectedResult should be frozen") + + +# --------------------------------------------------------------------------- # +# recommended_action # +# --------------------------------------------------------------------------- # + + +class TestRecommendedAction: + """``recommended_action`` collapses the orchestrator's go / wait / + skip decision into a single property. The check ordering is part + of the contract (skip wins over ask_user, ask_user wins over + process) — see the property docstring.""" + + def test_skip_when_no_main_video(self, tmp_path: Path) -> None: + # Folder with no video at all → main_video is None → skip. + folder = tmp_path / _MOVIE_NAME + folder.mkdir() + (folder / "readme.txt").write_text("hi") + + result = inspect_release(_MOVIE_NAME, folder, _KB, _RaisingProber()) + + assert result.main_video is None + assert result.recommended_action == "skip" + + def test_skip_when_media_type_other(self, tmp_path: Path) -> None: + # Folder with only non-video files (ISO) → media_type == "other" + # AND main_video is None (find_main_video filters by video ext). + # Both branches resolve to "skip"; this asserts the contract holds. + folder = tmp_path / _MOVIE_NAME + folder.mkdir() + (folder / "disc.iso").write_bytes(b"") + + result = inspect_release(_MOVIE_NAME, folder, _KB, _RaisingProber()) + + assert result.parsed.media_type == "other" + assert result.recommended_action == "skip" + + def test_ask_user_when_media_type_unknown(self, tmp_path: Path) -> None: + # Mixed video + non-video → detect_media_type returns "unknown". + folder = tmp_path / _MOVIE_NAME + folder.mkdir() + (folder / "movie.mkv").write_bytes(b"") + (folder / "extras.iso").write_bytes(b"") + + result = inspect_release( + _MOVIE_NAME, folder, _KB, _StubProber(_media_info_1080p_h264()) + ) + + assert result.parsed.media_type == "unknown" + assert result.recommended_action == "ask_user" + + def test_ask_user_when_path_of_pain_road(self, tmp_path: Path) -> None: + # Malformed name (forbidden chars) → road == "path_of_pain". + name = "garbage@#%name" + folder = tmp_path / "release" + folder.mkdir() + (folder / "movie.mkv").write_bytes(b"") + + result = inspect_release( + name, folder, _KB, _StubProber(_media_info_1080p_h264()) + ) + + assert result.report.road == "path_of_pain" + # main_video is found but the road still flags uncertainty. + assert result.main_video is not None + assert result.recommended_action == "ask_user" + + def test_process_for_confident_movie(self, tmp_path: Path) -> None: + folder = tmp_path / _MOVIE_NAME + folder.mkdir() + (folder / "movie.mkv").write_bytes(b"") + + result = inspect_release( + _MOVIE_NAME, folder, _KB, _StubProber(_media_info_1080p_h264()) + ) + + assert result.parsed.media_type == "movie" + assert result.report.road in ("easy", "shitty") + assert result.recommended_action == "process" + + def test_process_for_confident_tv_show(self, tmp_path: Path) -> None: + folder = tmp_path / _TV_NAME + folder.mkdir() + (folder / "episode.mkv").write_bytes(b"") + + result = inspect_release( + _TV_NAME, folder, _KB, _StubProber(_media_info_1080p_h264()) + ) + + assert result.parsed.media_type == "tv_show" + assert result.recommended_action == "process"