Feat/opt

[tiehongji]

No description provided.

[cuericlee]

ssues & suggestions

Critical: audio.py — probe_audio_metadata is missing from TOOL_NAMES

audio.py defines TOOL_NAMES = ['probe_audio_metadata', 'separate_voice'], and probe_audio_metadata is indeed registered as a tool, but the register_tools function in audio.py only implements separate_voice. The probe_audio_metadata tool registration is missing from audio.py.

Looking at the diff, the probe_audio_metadata tool appears to be defined inside video.py instead, but listed in audio.py's TOOL_NAMES. This means:

• register_domain_tools("audio", TOOL_NAMES) will look for probe_audio_metadata in the wrong domain file.
• The tool likely won't be discoverable under the audio domain.

Fix: Move probe_audio_metadata to video.py's TOOL_NAMES or implement the tool in audio.py.

Bug: format keyword shadows built-in

async def matte_greenscreen_video(
    video_url: str = Field(...),
    format: Optional[str] = Field('WEBM', ...),  # shadows built-in format()

format is a Python built-in. Using it as a parameter name shadows the built-in. Rename to output_format for consistency with good Python practices.

Same issue in matte_portrait_video.

Minor: * separator followed by ctx without kwargs

Several tools use:

    *,
    ctx: Context,
) -> dict:

The keyword-only separator * is followed only by ctx. Since ctx is injected by the MCP framework and not passed by the caller, this is functionally fine, but the * is unnecessary when there's only one keyword-only parameter. It's not a bug, but it's inconsistent with the rest of the codebase which doesn't use this pattern. Consider whether it adds value.

Style: Long lines

Several client.call(...) invocations are very long single-line calls (e.g., asr_subtitles passes 8+ keyword arguments in one line). Consider wrapping for readability — though this is consistent with the existing codebase's style, so it's a minor point.

Observation: No __init__.py or cli.py

The PR doesn't include:

• An __init__.py for the mcp_tools package
• A cli.py entry point
• A pyproject.toml or setup.py

These may be added in a separate PR or the package may be wired in differently. Worth confirming that the entry point exists somewhere.

Security: video_url / audio_url parameters

All tools accept arbitrary HTTP/HTTPS URLs with no validation beyond "must be publicly accessible." This is standard for the existing codebase, but worth noting as a consideration for SSRF mitigation if the server processes untrusted user input.

---

Summary

Category | Count -- | -- Bugs | 1 — probe_audio_metadata mis-assigned to audio domain Shadowed built-in | 2 — format parameter in two matte tools Style | Minor — long lines, optional * separator Missing pieces | No CLI entry point or package config in this PR

The PR is a solid addition with a clear structure. The main thing to fix is the probe_audio_metadata domain assignment — it's registered in video.py but listed under audio's TOOL_NAMES, which means it won't be properly discoverable via domain-based tool registration.

[cuericlee]

/lgtm