Skip to content

fix(clip_manager): retire argparse CLI in favor of corridorkey#236

Open
mountarreat wants to merge 1 commit intonikopueringer:mainfrom
mountarreat:fix/stale-clip-manager-cli
Open

fix(clip_manager): retire argparse CLI in favor of corridorkey#236
mountarreat wants to merge 1 commit intonikopueringer:mainfrom
mountarreat:fix/stale-clip-manager-cli

Conversation

@mountarreat
Copy link
Copy Markdown
Contributor

What does this change?

clip_manager.py still shipped an argparse __main__ block that
predates the Typer migration in PR #67. The wizard action
immediately raised
NotImplementedError("interactive_wizard is not yet implemented")
at clip_manager.py:1046, because the wizard implementation had
already moved to corridorkey_cli.py::interactive_wizard in
commit e54abc8. The other three actions (list,
generate_alphas, run_inference) still worked but were
redundant with the corridorkey console script.

The README.md also actively documented the stale invocations:

uv run python clip_manager.py --action wizard --win_path "V:\..."
uv run python clip_manager.py --action run_inference --device cpu
uv run python clip_manager.py --action list 2>&1 | grep ...
CORRIDORKEY_BACKEND=mlx uv run python clip_manager.py --action run_inference

Any user following the README to run the wizard through
clip_manager.py hit the NotImplementedError immediately. The
README also had two uv run python corridorkey_cli.py wizard --win_path ... stanzas that combined a non-console-script
invocation with flags that never existed on the Typer wizard
(--win_path was an argparse artifact; the Typer wizard takes
path as a positional argument).

This PR:

  • Replaces the argparse __main__ block in clip_manager.py
    (lines 1006-1046) with a small deprecation stub that prints
    a migration guide to stderr and exits non-zero. Users with
    existing scripts or cron jobs that invoke
    python clip_manager.py --action ... now see a clear
    explanation and a one-to-one mapping to corridorkey
    subcommands instead of the old NotImplementedError.
  • Removes the now-unused import argparse from line 3.
  • Rewrites six stale README.md stanzas to use the
    corridorkey console script and the correct Typer syntax.
  • Updates the backend override section header from
    "Override via CLI flag (corridorkey_cli.py)" to
    "Override via --backend flag (on run-inference)", so
    the section reflects reality (the flag is a run-inference
    option, not a global).

clip_manager.py remains the pipeline/library module for
generate_alphas, run_inference, run_videomama,
scan_clips, etc. Only the script entry point is retired.
docs/LLM_HANDOVER.md prose still refers to clip_manager.py
as "the user-facing Command Line Wizard"; that drift belongs
to the broader documentation-consolidation work and is
deliberately out of scope for this PR.

How was it tested?

  • uv run ruff format --check: clean
  • uv run ruff check: clean
  • uv run pytest -q --tb=short -m "not gpu": 363 passed, 1
    skipped (MLX), 4 deselected (GPU), 2 warnings. Same count
    as upstream. Both warnings are pre-existing on main and
    not introduced here.
  • uv run python clip_manager.py: prints the migration guide
    to stderr, exits 2.
  • uv run corridorkey --help: exits 0.
  • Typer command and option syntax for each rewritten README
    command cross-checked against corridorkey_cli.py source.

Checklist

  • uv run pytest passes
  • uv run ruff check passes
  • uv run ruff format --check passes

`clip_manager.py` still shipped an argparse `__main__` block that
predates the Typer migration in PR nikopueringer#67. The `wizard` action
immediately raised `NotImplementedError("interactive_wizard is
not yet implemented")` (`clip_manager.py:1046`) because the
wizard implementation had already moved to
`corridorkey_cli.py::interactive_wizard` in commit `e54abc8`. The
other three actions (`list`, `generate_alphas`, `run_inference`)
still worked but were redundant with the `corridorkey` console
script defined by `pyproject.toml::[project.scripts]`.

Meanwhile, `README.md` actively documented the stale invocations:

    uv run python clip_manager.py --action wizard --win_path "V:\..."
    uv run python clip_manager.py --action run_inference --device cpu
    uv run python clip_manager.py --action list 2>&1 | grep ...
    CORRIDORKEY_BACKEND=mlx uv run python clip_manager.py --action run_inference

Any user following the README to invoke the wizard through
`clip_manager.py` hit the NotImplementedError immediately. The
README also had two `uv run python corridorkey_cli.py wizard
--win_path ...` stanzas that combined a non-console-script
invocation with flags that never existed on the Typer wizard
(`--win_path` was an argparse artifact; the Typer wizard takes
`path` as a positional argument).

This commit:

- Replaces the argparse `__main__` block in `clip_manager.py`
  (lines 1006-1046) with a short deprecation stub that prints a
  migration guide to stderr and exits non-zero (`sys.exit(2)`,
  matching argparse's own usage-error convention). Users with
  existing scripts or cron jobs that invoke
  `python clip_manager.py --action ...` now see a clear
  explanation and a one-to-one mapping to `corridorkey`
  subcommands instead of the old NotImplementedError.
- Removes the now-unused `import argparse` from line 3.
- Rewrites six README stanzas to use the `corridorkey` console
  script and the correct Typer syntax (positional `path`,
  `run-inference` with a hyphen, `--device` as a global flag
  before the subcommand, `--backend` only on `run-inference`).
- Changes the backend override section header from "Override
  via CLI flag (corridorkey_cli.py)" to "Override via
  `--backend` flag (on `run-inference`)", so the section
  reflects reality (the flag is a `run-inference` option, not
  a global).

Intentionally not changed:

- `clip_manager.py` remains the pipeline/library module for
  `generate_alphas`, `run_inference`, `run_videomama`,
  `scan_clips`, etc. Only the script entry point is retired.
- `docs/LLM_HANDOVER.md` prose still describes `clip_manager.py`
  as "the user-facing Command Line Wizard". That drift belongs
  to the separate documentation-consolidation work and is out
  of this PR's scope.

Verified locally on Windows with Python 3.13.11:

- `uv run ruff format --check`: clean
- `uv run ruff check`: clean
- `uv run pytest -q --tb=short -m "not gpu"`: 363 passed, 1
  skipped (MLX), 4 deselected (GPU), 2 warnings. Same count as
  upstream. Both warnings are pre-existing (pytest `env` config
  warning addressed on a separate branch; torch autocast
  warning on a no-CUDA machine).
- `uv run python clip_manager.py`: prints the migration guide
  to stderr, exits 2.
- `uv run corridorkey --help`: exits 0.
@mountarreat mountarreat marked this pull request as ready for review April 18, 2026 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant