Streamlines imports and app start

2026-04-11 14:57:01 +02:00
parent d19e69990a
commit 71553aad32
6 changed files with 199 additions and 79 deletions
--- a/SCRATCHPAD.md
+++ b/SCRATCHPAD.md
@@ -9,6 +9,8 @@
 - The biggest near-term wins are in startup cost, repeated subprocess work, repeated database query patterns, and general repo hygiene.
 - This list is intentionally optimization-oriented rather than bug-oriented. Some items below also improve correctness or maintainability, but they were selected because they can reduce runtime cost, operator friction, or iteration overhead.
 - A first modern integration slice now exists under [`tests/integration/subtrack_mapping`](/home/osgw/.local/src/codex/ffx/tests/integration/subtrack_mapping). Remaining test-suite cleanup is now mostly about migrating and shrinking the legacy harness surface under [`tests/legacy`](/home/osgw/.local/src/codex/ffx/tests/legacy).
+- The CLI root now lazy-loads heavy runtime dependencies so lightweight commands such as `version`, `help`, `configure_workstation`, and `upgrade` stay import-light.
+- Shared CLI defaults for container/output tokens now live outside [`src/ffx/ffx_controller.py`](/home/osgw/.local/src/codex/ffx/src/ffx/ffx_controller.py), and a focused unit test locks in the lazy-import contract.
 - FFX logger setup now reuses named handlers, and fallback logger access no longer mutates handlers in ordinary constructors and helpers.
 - The process wrapper now uses `subprocess.run(...)` with centralized command formatting plus stable timeout and missing-command error mapping.
 - Active ORM controllers now use single-query accessors instead of paired `count()` plus `first()` lookups.
@@ -17,8 +19,8 @@
 ## Focused Snapshot

 - Highest-leverage application optimizations:
-  - Lazy-load CLI command dependencies so lightweight commands do not import most of the app.
  - Collapse repeated `ffprobe` calls into a single probe result per source file.
+  - Revisit crop detection cost after the probe path is consolidated.

 - Highest-leverage repo and workflow optimizations:
  - Consolidate setup and upgrade tooling to reduce overlapping shell-script responsibilities.
@@ -26,16 +28,7 @@

 ## Optimization Candidates

-1. CLI startup and import cost
- [`src/ffx/cli.py`](/home/osgw/.local/src/codex/ffx/src/ffx/cli.py) imports a large portion of the application at module import time, even for cheap commands such as `version`, `help`, `configure_workstation`, and `upgrade`.
- Optimization:
-  - Move heavy imports into the commands that actually need them.
-  - Keep the CLI root importable with only core stdlib and Click dependencies.
- Expected value:
-  - Faster startup for scripting and tooling commands.
-  - Less coupling between maintenance commands and the runtime stack.
-
-2. Media probing does two separate `ffprobe` subprocesses per file
+1. Media probing does two separate `ffprobe` subprocesses per file
 - [`src/ffx/file_properties.py`](/home/osgw/.local/src/codex/ffx/src/ffx/file_properties.py) calls `ffprobe` once for format data and once for stream data.
 - Optimization:
  - Use one probe call that requests both format and streams.
@@ -44,7 +37,7 @@
  - Less subprocess overhead.
  - Faster inspect and convert flows.

-3. Crop detection is always a full extra ffmpeg scan
+2. Crop detection is always a full extra ffmpeg scan
 - [`src/ffx/file_properties.py`](/home/osgw/.local/src/codex/ffx/src/ffx/file_properties.py) runs a dedicated `ffmpeg -vf cropdetect` pass for each file when crop detection is requested.
 - Optimization:
  - Cache crop results for repeated runs on the same source.
@@ -52,7 +45,7 @@
 - Expected value:
  - Lower latency on repeated experimentation.

-4. Tooling overlap and naming drift
+3. Tooling overlap and naming drift
 - There are still overlapping workstation-setup entrypoints across [`tools/configure_workstation.sh`](/home/osgw/.local/src/codex/ffx/tools/configure_workstation.sh), [`tools/setup.sh`](/home/osgw/.local/src/codex/ffx/tools/setup.sh), and newer CLI maintenance commands.
 - Optimization:
  - Decide which scripts remain canonical.
@@ -62,7 +55,7 @@
  - Less operator confusion.
  - Fewer duplicated procedures to maintain.

-5. Placeholder UI surfaces should either ship or disappear
+4. Placeholder UI surfaces should either ship or disappear
 - [`src/ffx/help_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/help_screen.py) and [`src/ffx/settings_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/settings_screen.py) are placeholders.
 - Optimization:
  - Either remove them from the active UI surface or complete them.
@@ -71,7 +64,7 @@
  - Leaner interface.
  - Lower UX ambiguity.

-6. Large Textual screens repeat configuration and controller loading
+5. Large Textual screens repeat configuration and controller loading
 - Screens such as [`src/ffx/media_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/media_details_screen.py), [`src/ffx/pattern_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/pattern_details_screen.py), and [`src/ffx/show_details_screen.py`](/home/osgw/.local/src/codex/ffx/src/ffx/show_details_screen.py) repeat setup patterns and local metadata filtering extraction.
 - Optimization:
  - Extract a shared screen base or helper for common config/controller/bootstrap logic.
@@ -80,7 +73,7 @@
  - Lower maintenance overhead.
  - Easier UI iteration.

-7. Several helper functions are unfinished or dead-weight
+6. Several helper functions are unfinished or dead-weight
 - [`src/ffx/helper.py`](/home/osgw/.local/src/codex/ffx/src/ffx/helper.py) contains `permutateList(...): pass`.
 - There are many combinator and conversion placeholders across tests and migrations.
 - Optimization:
@@ -90,7 +83,7 @@
  - Smaller mental model.
  - Less time spent re-evaluating inactive paths.

-8. Test suite shape is expensive to understand and likely expensive to run
+7. Test suite shape is expensive to understand and likely expensive to run
 - The project still carries a large legacy matrix of combinator files under [`tests/legacy`](/home/osgw/.local/src/codex/ffx/tests/legacy), several placeholder `pass` implementations, and at least one suspicious filename with an embedded space: [`tests/legacy/disposition_combinator_2_3 .py`](/home/osgw/.local/src/codex/ffx/tests/legacy/disposition_combinator_2_3 .py).
 - A first focused replacement slice now exists in [`tests/integration/subtrack_mapping/test_cli_bundle.py`](/home/osgw/.local/src/codex/ffx/tests/integration/subtrack_mapping/test_cli_bundle.py), so the remaining work is migration and consolidation rather than creating the modern test shape from scratch.
 - Optimization:
@@ -101,7 +94,7 @@
  - Faster contributor onboarding.
  - Easier CI adoption later.

-9. Process resource limiting semantics could be clearer
+8. Process resource limiting semantics could be clearer
 - [`src/ffx/process.py`](/home/osgw/.local/src/codex/ffx/src/ffx/process.py) prepends `nice` and `cpulimit` directly when values are set.
 - Optimization:
  - Validate and document effective behavior for combined `nice` + `cpulimit`.
@@ -110,24 +103,16 @@
  - Fewer surprises in production-like runs.
  - Easier support for user-reported performance behavior.

-10. Import-time dependency coupling makes maintenance commands brittle
- Even after recent CLI maintenance additions, the top-level CLI module still imports most application modules before Click dispatch.
+9. Regex and string utility cleanup
+- [`src/ffx/helper.py`](/home/osgw/.local/src/codex/ffx/src/ffx/helper.py) still has repeated string-replacement churn in filename/TMDB normalization helpers, and regex handling in helpers is easy to regress quietly.
 - Optimization:
-  - Push imports for ORM, Textual, TMDB, ffmpeg helpers, and descriptors behind the commands that actually need them.
- Expected value:
-  - Maintenance commands such as setup and upgrade stay usable when optional runtime dependencies are broken.
-  - Better separation between media runtime code and maintenance tooling.
-
-11. Regex and string utility cleanup
- [`src/ffx/helper.py`](/home/osgw/.local/src/codex/ffx/src/ffx/helper.py) still emits a `SyntaxWarning` for `RICH_COLOR_PATTERN`.
- Optimization:
-  - Convert regex literals to raw strings where appropriate.
+  - Keep regex literals raw and centralized where appropriate.
  - Review filename and TMDB substitution helpers for repeated string churn.
 - Expected value:
  - Cleaner runtime output.
  - Less warning noise during dry-run maintenance commands.

-12. Database startup always runs schema creation and version checks
+10. Database startup always runs schema creation and version checks
 - [`src/ffx/database.py`](/home/osgw/.local/src/codex/ffx/src/ffx/database.py) runs `Base.metadata.create_all(...)` and version checks every time a DB-backed context is created.
 - Optimization:
  - Measure startup cost and consider separating bootstrapping from ordinary command execution.
@@ -151,9 +136,9 @@

 1. Triage the list into quick wins, medium refactors, and long-horizon cleanup.
 2. Tackle the cheapest high-impact items first:
-   - regex raw-string warning cleanup,
   - single-call `ffprobe` refactor.
-3. Decide whether maintenance/tooling command imports should be split from media-runtime imports before adding more CLI maintenance surface.
+   - crop detection sampling or caching pass.
+3. Decide which setup and upgrade entrypoints stay canonical before adding more maintenance surface.

 ## Delete When

--- a/src/ffx/cli.py
+++ b/src/ffx/cli.py
@@ -1,6 +1,9 @@
 #! /usr/bin/python3

+from __future__ import annotations
+
 import os, sys, click, time, shutil, subprocess
+from typing import TYPE_CHECKING

 # Allow direct execution via `python src/ffx/cli.py` by preferring the package
 # root on sys.path.
@@ -10,42 +13,24 @@ if __package__ in (None, ''):
    sys.path = [p for p in sys.path if os.path.abspath(p) != os.path.abspath(script_dir)]
    sys.path.insert(0, package_root)

-from ffx.configuration_controller import ConfigurationController
-
-from ffx.file_properties import FileProperties
-
-from ffx.ffx_app import FfxApp
-from ffx.ffx_controller import FfxController
-from ffx.tmdb_controller import TmdbController
-
-from ffx.database import databaseContext
+from ffx.constants import (
+    DEFAULT_AC3_BANDWIDTH,
+    DEFAULT_CONTAINER_EXTENSION,
+    DEFAULT_CONTAINER_FORMAT,
+    DEFAULT_DTS_BANDWIDTH,
+    DEFAULT_STEREO_BANDWIDTH,
+    DEFAULT_VIDEO_ENCODER_LABEL,
+    FFMPEG_COMMAND_TOKENS,
+    SUPPORTED_INPUT_FILE_EXTENSIONS,
+    VERSION,
+)

+if TYPE_CHECKING:
    from ffx.media_descriptor import MediaDescriptor
    from ffx.track_descriptor import TrackDescriptor
-from ffx.show_descriptor import ShowDescriptor

-from ffx.track_type import TrackType
-from ffx.video_encoder import VideoEncoder
-from ffx.track_disposition import TrackDisposition
-from ffx.track_codec import TrackCodec
+LIGHTWEIGHT_COMMANDS = {None, 'version', 'help', 'configure_workstation', 'upgrade'}

-from ffx.process import executeProcess
-from ffx.helper import filterFilename, substituteTmdbFilename
-from ffx.helper import getEpisodeFileBasename
-
-from ffx.constants import DEFAULT_STEREO_BANDWIDTH, DEFAULT_AC3_BANDWIDTH, DEFAULT_DTS_BANDWIDTH, DEFAULT_7_1_BANDWIDTH
-
-from ffx.filter.quality_filter import QualityFilter
-from ffx.filter.preset_filter import PresetFilter
-
-from ffx.filter.crop_filter import CropFilter
-from ffx.filter.nlmeans_filter import NlmeansFilter
-from ffx.filter.deinterlace_filter import DeinterlaceFilter
-
-from ffx.constants import VERSION
-
-from ffx.shifted_season_controller import ShiftedSeasonController
-from ffx.logging_utils import configure_ffx_logger


@click.group()
@@ -58,11 +43,18 @@ def ffx(ctx, database_file, verbose, dry_run):

    ctx.obj = {}

-    if ctx.invoked_subcommand in ('configure_workstation', 'upgrade'):
+    if ctx.resilient_parsing:
+        return
+
+    if ctx.invoked_subcommand in LIGHTWEIGHT_COMMANDS:
        ctx.obj['dry_run'] = dry_run
        ctx.obj['verbosity'] = verbose
        return

+    from ffx.configuration_controller import ConfigurationController
+    from ffx.database import databaseContext
+    from ffx.logging_utils import configure_ffx_logger
+
    ctx.obj['config'] = ConfigurationController()

    ctx.obj['database'] = databaseContext(databasePath=database_file
@@ -184,6 +176,7 @@ def upgrade(ctx, branch):
@click.pass_context
@click.argument('filename', nargs=1)
 def inspect(ctx, filename):
+    from ffx.ffx_app import FfxApp

    ctx.obj['command'] = 'inspect'
    ctx.obj['arguments'] = {}
@@ -196,7 +189,7 @@ def inspect(ctx, filename):
 def getUnmuxSequence(trackDescriptor: TrackDescriptor, sourcePath, targetPrefix, targetDirectory = ''):

    # executable and input file
-    commandTokens = FfxController.COMMAND_TOKENS + ['-i', sourcePath]
+    commandTokens = list(FFMPEG_COMMAND_TOKENS) + ['-i', sourcePath]

    trackType = trackDescriptor.getType()

@@ -237,6 +230,10 @@ def unmux(ctx,
            subtitles_only,
            nice,
            cpu):
+    from ffx.file_properties import FileProperties
+    from ffx.process import executeProcess
+    from ffx.track_disposition import TrackDisposition
+    from ffx.track_type import TrackType

    existingSourcePaths = [p for p in paths if os.path.isfile(p)]
    ctx.obj['logger'].debug(f"\nUnmuxing {len(existingSourcePaths)} files")
@@ -307,6 +304,7 @@ def cropdetect(ctx,
            paths,
            nice,
            cpu):
+    from ffx.file_properties import FileProperties

    existingSourcePaths = [p for p in paths if os.path.isfile(p)]
    ctx.obj['logger'].debug(f"\nUnmuxing {len(existingSourcePaths)} files")
@@ -333,6 +331,7 @@ def cropdetect(ctx,
@click.pass_context

 def shows(ctx):
+    from ffx.ffx_app import FfxApp

    ctx.obj['command'] = 'shows'

@@ -341,6 +340,8 @@ def shows(ctx):


 def checkUniqueDispositions(context, mediaDescriptor: MediaDescriptor):
+    from ffx.track_disposition import TrackDisposition
+    from ffx.track_type import TrackType
    
    # Check for multiple default or forced dispositions if not set by user input or database requirements
    #
@@ -390,7 +391,7 @@ def checkUniqueDispositions(context, mediaDescriptor: MediaDescriptor):

@click.option('-l', '--label', type=str, default='', help='Label to be used as filename prefix')

-@click.option('-v', '--video-encoder', type=str, default=FfxController.DEFAULT_VIDEO_ENCODER, help=f"Target video encoder (vp9, av1, h264 or copy)", show_default=True)
+@click.option('-v', '--video-encoder', type=str, default=DEFAULT_VIDEO_ENCODER_LABEL, help=f"Target video encoder (vp9, av1, h264 or copy)", show_default=True)

@click.option('-q', '--quality', type=str, default="", help=f"Quality settings to be used with VP9/H264 encoder")
@click.option('-p', '--preset', type=str, default="", help=f"Quality preset to be used with AV1 encoder")
@@ -507,6 +508,20 @@ def convert(ctx,
    Filename extensions will be changed appropriately.
    Suffices will we appended to filename in case of multiple created files
    or if the filename has not changed."""
+    from ffx.ffx_controller import FfxController
+    from ffx.file_properties import FileProperties
+    from ffx.filter.crop_filter import CropFilter
+    from ffx.filter.deinterlace_filter import DeinterlaceFilter
+    from ffx.filter.nlmeans_filter import NlmeansFilter
+    from ffx.filter.preset_filter import PresetFilter
+    from ffx.filter.quality_filter import QualityFilter
+    from ffx.helper import filterFilename, getEpisodeFileBasename, substituteTmdbFilename
+    from ffx.shifted_season_controller import ShiftedSeasonController
+    from ffx.show_descriptor import ShowDescriptor
+    from ffx.tmdb_controller import TmdbController
+    from ffx.track_codec import TrackCodec
+    from ffx.track_disposition import TrackDisposition
+    from ffx.video_encoder import VideoEncoder

    startTime = time.perf_counter()

@@ -519,8 +534,8 @@ def convert(ctx,
        targetFormat = ''
        targetExtension = 'mkv'
    else:
-        targetFormat = FfxController.DEFAULT_FILE_FORMAT
-        targetExtension = FfxController.DEFAULT_FILE_EXTENSION
+        targetFormat = DEFAULT_CONTAINER_FORMAT
+        targetExtension = DEFAULT_CONTAINER_EXTENSION

    context['use_tmdb'] = not no_tmdb
    context['use_pattern'] = not no_pattern
@@ -540,7 +555,7 @@ def convert(ctx,
        context['subtitle_prefix'] = subtitle_prefix


-    existingSourcePaths = [p for p in paths if os.path.isfile(p) and p.split('.')[-1] in FfxController.INPUT_FILE_EXTENSIONS]
+    existingSourcePaths = [p for p in paths if os.path.isfile(p) and p.split('.')[-1] in SUPPORTED_INPUT_FILE_EXTENSIONS]


    # CLI Overrides
--- a/src/ffx/constants.py
+++ b/src/ffx/constants.py
@@ -4,6 +4,13 @@ DATABASE_VERSION = 2
 DEFAULT_QUALITY = 32
 DEFAULT_AV1_PRESET = 5

+DEFAULT_VIDEO_ENCODER_LABEL = "vp9"
+DEFAULT_CONTAINER_FORMAT = "webm"
+DEFAULT_CONTAINER_EXTENSION = "webm"
+SUPPORTED_INPUT_FILE_EXTENSIONS = ("mkv", "mp4", "avi", "flv", "webm")
+FFMPEG_COMMAND_TOKENS = ("ffmpeg", "-y")
+FFMPEG_NULL_OUTPUT_TOKENS = ("-f", "null", "/dev/null")
+
 DEFAULT_STEREO_BANDWIDTH    = "112"
 DEFAULT_AC3_BANDWIDTH       = "256"
 DEFAULT_DTS_BANDWIDTH       = "320"
--- a/src/ffx/ffx_controller.py
+++ b/src/ffx/ffx_controller.py
@@ -10,7 +10,16 @@ from ffx.track_codec import TrackCodec
 from ffx.video_encoder import VideoEncoder
 from ffx.process import executeProcess

-from ffx.constants import DEFAULT_cut_start, DEFAULT_cut_length
+from ffx.constants import (
+    DEFAULT_CONTAINER_EXTENSION,
+    DEFAULT_CONTAINER_FORMAT,
+    DEFAULT_VIDEO_ENCODER_LABEL,
+    DEFAULT_cut_start,
+    DEFAULT_cut_length,
+    FFMPEG_COMMAND_TOKENS,
+    FFMPEG_NULL_OUTPUT_TOKENS,
+    SUPPORTED_INPUT_FILE_EXTENSIONS,
+)

 from ffx.filter.quality_filter import QualityFilter
 from ffx.filter.preset_filter import PresetFilter
@@ -21,17 +30,17 @@ from ffx.model.pattern import Pattern

 class FfxController():

-    COMMAND_TOKENS = ['ffmpeg', '-y']
-    NULL_TOKENS = ['-f', 'null', '/dev/null'] # -f null /dev/null
+    COMMAND_TOKENS = list(FFMPEG_COMMAND_TOKENS)
+    NULL_TOKENS = list(FFMPEG_NULL_OUTPUT_TOKENS) # -f null /dev/null

    TEMP_FILE_NAME = "ffmpeg2pass-0.log"

-    DEFAULT_VIDEO_ENCODER = VideoEncoder.VP9.label()
+    DEFAULT_VIDEO_ENCODER = DEFAULT_VIDEO_ENCODER_LABEL

-    DEFAULT_FILE_FORMAT = 'webm'
-    DEFAULT_FILE_EXTENSION = 'webm'
+    DEFAULT_FILE_FORMAT = DEFAULT_CONTAINER_FORMAT
+    DEFAULT_FILE_EXTENSION = DEFAULT_CONTAINER_EXTENSION

-    INPUT_FILE_EXTENSIONS = ['mkv', 'mp4', 'avi', 'flv', 'webm']
+    INPUT_FILE_EXTENSIONS = list(SUPPORTED_INPUT_FILE_EXTENSIONS)
    
    CHANNEL_MAP_5_1 = 'FL-FL|FR-FR|FC-FC|LFE-LFE|SL-BL|SR-BR:5.1'

--- a/src/ffx/helper.py
+++ b/src/ffx/helper.py
@@ -16,7 +16,7 @@ DIFF_REMOVED_KEY = 'removed'
 DIFF_CHANGED_KEY = 'changed'
 DIFF_UNCHANGED_KEY = 'unchanged'

-RICH_COLOR_PATTERN = '\[[a-z_]+\](.+)\[\/[a-z_]+\]'
+RICH_COLOR_PATTERN = '\\[[a-z_]+\\](.+)\\[\\/[a-z_]+\\]'


 def dictDiff(a : dict, b : dict, ignoreKeys: list = [], removeKeys: list = []):
--- a/tests/unit/test_cli_lazy_imports.py
+++ b/tests/unit/test_cli_lazy_imports.py
@@ -0,0 +1,104 @@
+from __future__ import annotations
+
+import json
+from pathlib import Path
+import subprocess
+import sys
+import textwrap
+import unittest
+
+
+REPO_ROOT = Path(__file__).resolve().parents[2]
+SRC_ROOT = REPO_ROOT / "src"
+HEAVY_MODULES = [
+    "ffx.configuration_controller",
+    "ffx.database",
+    "ffx.ffx_app",
+    "ffx.ffx_controller",
+    "ffx.file_properties",
+    "ffx.tmdb_controller",
+]
+
+
+class CliLazyImportTests(unittest.TestCase):
+    def run_python(self, code: str) -> dict:
+        completed = subprocess.run(
+            [sys.executable, "-c", code],
+            capture_output=True,
+            cwd=REPO_ROOT,
+            text=True,
+        )
+        if completed.returncode != 0:
+            self.fail(
+                "Python helper failed\n"
+                f"STDOUT:\n{completed.stdout}\n"
+                f"STDERR:\n{completed.stderr}"
+            )
+        return json.loads(completed.stdout)
+
+    def test_importing_cli_keeps_runtime_modules_unloaded(self):
+        result = self.run_python(
+            textwrap.dedent(
+                f"""
+                import json
+                import sys
+
+                sys.path.insert(0, {str(SRC_ROOT)!r})
+
+                import ffx.cli
+
+                print(json.dumps({{
+                    "modules": {{
+                        module_name: module_name in sys.modules
+                        for module_name in {HEAVY_MODULES!r}
+                    }},
+                }}))
+                """
+            )
+        )
+
+        self.assertTrue(
+            all(not is_loaded for is_loaded in result["modules"].values()),
+            result["modules"],
+        )
+
+    def test_lightweight_configure_workstation_command_stays_light(self):
+        result = self.run_python(
+            textwrap.dedent(
+                f"""
+                import json
+                import sys
+                from click.testing import CliRunner
+
+                sys.path.insert(0, {str(SRC_ROOT)!r})
+
+                import ffx.cli
+
+                runner = CliRunner()
+                invoke_result = runner.invoke(
+                    ffx.cli.ffx,
+                    ["--dry-run", "configure_workstation", "--check"],
+                )
+                if invoke_result.exit_code != 0:
+                    raise SystemExit(invoke_result.output)
+
+                print(json.dumps({{
+                    "output": invoke_result.output,
+                    "modules": {{
+                        module_name: module_name in sys.modules
+                        for module_name in {HEAVY_MODULES!r}
+                    }},
+                }}))
+                """
+            )
+        )
+
+        self.assertIn("configure_workstation.sh --check", result["output"])
+        self.assertTrue(
+            all(not is_loaded for is_loaded in result["modules"].values()),
+            result["modules"],
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()