From ec8384a0c157961b08eed8ace95a691dc0cadaf4 Mon Sep 17 00:00:00 2001 From: Spencer Qian Date: Fri, 24 Jul 2026 16:27:41 -0700 Subject: [PATCH 1/2] feat(cli): add --segments to the segment-capable commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit segments is the only structured parameter the API takes, so it was skipped when the CLI was written. Both SDKs accept it; neither CLI exposed it. The flag takes inline JSON, @file, or @- for stdin — the curl / gh / aws convention. Validation is shape-only: parseable JSON, a non-empty array of objects, required fields present with the right types. The server's semantic rules (first segment at 0, minimum spacing, the label enum, item caps) are left to the API's 422, since a copy of them here would drift. Music commands (text-to-music, video-to-music) take {start, prompt, label?}; SFX commands (video-to-sfx, video-to-sound, video-to-video-sound) take {start, end, prompt}. Passing one shape to a command that takes the other is the predictable mistake, so errors name the expected shape and the keys that were actually given. Keys belonging to neither shape are forwarded untouched, so a field added to the API needs no CLI release. video-to-video-sfx is the sixth segment-capable endpoint in the SDK but has no CLI command yet, so only five commands gained the flag here. --- sonilo-cli/README.md | 33 ++++ sonilo-cli/pyproject.toml | 2 +- sonilo-cli/src/sonilo_cli/__init__.py | 2 +- sonilo-cli/src/sonilo_cli/__main__.py | 165 ++++++++++++++++++- sonilo-cli/tests/test_cli.py | 219 ++++++++++++++++++++++++++ 5 files changed, 415 insertions(+), 6 deletions(-) diff --git a/sonilo-cli/README.md b/sonilo-cli/README.md index f0eb305..868d160 100644 --- a/sonilo-cli/README.md +++ b/sonilo-cli/README.md @@ -22,6 +22,7 @@ or pass `--api-key sk-...` on any command. sonilo video-to-music --video clip.mp4 --prompt "tense synths" --format wav sonilo text-to-sfx --prompt "glass shattering on concrete" --duration 3 sonilo video-to-sfx --video clip.mp4 --output whoosh.wav + sonilo video-to-sfx --video clip.mp4 --segments @segments.json sonilo video-to-sound --video clip.mp4 \ --music-prompt "uplifting orchestral score" --sfx-prompt "match the on-screen action" sonilo video-to-video-sound --video clip.mp4 --music-prompt "tense synths" @@ -37,6 +38,36 @@ or pass `--api-key sk-...` on any command. - `text-to-sfx` / `video-to-sfx` are always async; `--format` accepts `wav|mp3|aac|flac`. - Output defaults to `./output.`; override with `--output`. +### Segments + +`--segments` scores a timeline instead of one whole-clip prompt. It takes a JSON array, in one of +three forms — inline, from a file, or from stdin: + + sonilo text-to-music --prompt "warm lo-fi piano" --duration 30 \ + --segments '[{"start":0,"label":"intro","prompt":"airy pads"}]' + sonilo video-to-sfx --video clip.mp4 --segments @segments.json + jq -c '.cues' storyboard.json | sonilo video-to-sfx --video clip.mp4 --segments @- + +A value starting with `@` names a source to read the JSON from, and `@-` reads standard input — the +same convention as `curl`, `gh` and `aws`. Anything else is parsed as JSON directly. + +The two segment shapes are **not** interchangeable: + +| Shape | Commands | Fields | +| --- | --- | --- | +| Music | `text-to-music`, `video-to-music` | `{start, prompt, label?}` | +| SFX | `video-to-sfx`, `video-to-sound`, `video-to-video-sound` | `{start, end, prompt}` | + +- `start` / `end` are seconds from the start of the track or clip. +- Passing one shape to a command that takes the other is rejected before any request is made, with + a message naming the shape that command expects. +- Only the shape is checked locally. Timing rules — the first segment starting at 0, minimum + spacing between segments, the `label` vocabulary, how many segments are allowed — are enforced by + the API, which answers with a `422` describing what it rejected. +- Keys the CLI does not recognise are forwarded as-is, so a newly added API field works without + upgrading the CLI. +- `text-to-sfx` takes no segments (its output is a single effect, not a timeline). + ### Combined soundtracks `video-to-sound` and `video-to-video-sound` score a clip with a music bed *and* sound effects in one @@ -51,6 +82,8 @@ they differ only in what comes back: `video-to-sound` writes the mixed **audio** --output soundtrack.wav --stem music --stem sfx - `--music-prompt` / `--sfx-prompt` steer the two layers separately; both are optional. +- `--segments` places individual effects on the timeline, in the SFX shape `{start, end, prompt}` — + see [Segments](#segments). - `--preserve-speech` keeps speech from the source video in the mix. - **Ducking is on by default** (music dips under speech). Pass `--no-ducking` to opt out — omitting the flag leaves the server default untouched. diff --git a/sonilo-cli/pyproject.toml b/sonilo-cli/pyproject.toml index 4982263..f3bd70e 100644 --- a/sonilo-cli/pyproject.toml +++ b/sonilo-cli/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "sonilo-cli" -version = "0.3.0" +version = "0.4.0" description = "Command-line interface for the Sonilo API: generate music and sound effects from text or video" readme = "README.md" license = "MIT" diff --git a/sonilo-cli/src/sonilo_cli/__init__.py b/sonilo-cli/src/sonilo_cli/__init__.py index ec754d1..b786da2 100644 --- a/sonilo-cli/src/sonilo_cli/__init__.py +++ b/sonilo-cli/src/sonilo_cli/__init__.py @@ -1,3 +1,3 @@ -__version__ = "0.3.0" +__version__ = "0.4.0" __all__ = ["__version__"] diff --git a/sonilo-cli/src/sonilo_cli/__main__.py b/sonilo-cli/src/sonilo_cli/__main__.py index 2142402..2fd648f 100644 --- a/sonilo-cli/src/sonilo_cli/__main__.py +++ b/sonilo-cli/src/sonilo_cli/__main__.py @@ -6,7 +6,7 @@ import sys import time from pathlib import Path -from typing import Any, Dict, List, NoReturn, Optional +from typing import Any, Dict, List, NamedTuple, NoReturn, Optional, Tuple from urllib.parse import urlparse from sonilo import Sonilo @@ -36,6 +36,138 @@ def _wrote(path: Any, size: int) -> None: print(f"Wrote {path} ({size:,} bytes)") +# --- --segments ---------------------------------------------------------- +# +# `segments` is the only structured parameter the API takes — every other +# flag on this CLI is a scalar. The value follows the curl / gh / aws +# convention for anything that can get long: a leading `@` makes the value a +# *source* to read from rather than the value itself, and `@-` means stdin. +# +# Validation here is deliberately shape-only. The server owns the semantic +# rules (first segment at 0, minimum spacing, the label enum, item caps) and +# a copy of them in the CLI would drift the moment the backend changes, so a +# request that is the right shape but the wrong content is left to earn its +# own 422. + +_STDIN = "@-" + + +class _SegmentShape(NamedTuple): + """One command's segment contract, as used for validation and messages.""" + + summary: str + """What a correct segment looks like, shown verbatim in errors.""" + required: Tuple[Tuple[str, str], ...] + optional: Tuple[Tuple[str, str], ...] + foreign: Tuple[str, ...] + """Fields that belong to the *other* shape — see _check_segment.""" + + +MUSIC_SHAPE = _SegmentShape( + summary="{start, prompt, label?}", + required=(("start", "number"), ("prompt", "string")), + optional=(("label", "string"),), + foreign=("end",), +) + +SFX_SHAPE = _SegmentShape( + summary="{start, end, prompt}", + required=(("start", "number"), ("end", "number"), ("prompt", "string")), + optional=(), + foreign=("label",), +) + +# JSON booleans are not numbers, but bool is an int subclass in Python, so +# `isinstance(True, int)` would let `{"start": true}` through. +_TYPE_CHECKS = { + "number": lambda v: isinstance(v, (int, float)) and not isinstance(v, bool), + "string": lambda v: isinstance(v, str), +} + + +def _describe(value: Any) -> str: + """Name a JSON value the way the error messages talk about it. For an + object this lists its keys, which is what makes a shape mix-up obvious.""" + if value is None: + return "null" + if isinstance(value, bool): + return "a boolean" + if isinstance(value, (int, float)): + return "a number" + if isinstance(value, str): + return "a string" + if isinstance(value, list): + return "an empty array" if not value else "an array" + if isinstance(value, dict): + return f"an object with keys {', '.join(value)}" if value else "an object with no keys" + return "an unsupported value" + + +def _read_segments_source(raw: str) -> Tuple[str, str]: + """Resolve a raw --segments value to (text, source name for errors).""" + if not raw.startswith("@"): + return raw, "--segments" + if raw == _STDIN: + return sys.stdin.read(), "standard input" + path = raw[1:] + if not path: + _fail("--segments @ needs a filename, e.g. --segments @segments.json (@- reads stdin)") + try: + return Path(path).read_text(), path + except OSError as exc: + _fail(f"could not read segments from {path}: {exc.strerror or exc}") + + +def _check_segment(item: Any, shape: _SegmentShape, command: str) -> None: + expected = f"{command} segments take {shape.summary}" + if not isinstance(item, dict): + _fail(f"{expected} — got {_describe(item)}, not an object") + # A key that belongs to the *other* shape is the tell for the one + # predictable mistake here — SFX-shaped segments on a music command, or + # the reverse — so it is reported instead of forwarded, even though it + # would otherwise look like a required field is merely missing. Keys that + # belong to neither shape pass through untouched, so a field added to the + # API later needs no CLI release. + if any(key in item for key in shape.foreign) or any( + field not in item for field, _ in shape.required + ): + _fail(f"{expected} — got {_describe(item)}") + for field, kind in shape.required + shape.optional: + if field in item and not _TYPE_CHECKS[kind](item[field]): + _fail(f"{expected} — got {field}={json.dumps(item[field])}, expected a {kind}") + + +def parse_segments( + raw: Optional[str], shape: _SegmentShape, command: str +) -> Optional[List[Dict[str, Any]]]: + """Read, parse and shape-check one --segments value. + + `raw` is the flag as typed (inline JSON, `@file`, or `@-`). Returns None + when the flag was not given, so the field stays out of the request + entirely rather than being sent as an empty list. + """ + if raw is None: + return None + text, source = _read_segments_source(raw) + try: + value = json.loads(text) + except ValueError as exc: + _fail(f"could not parse segments from {source}: {exc}") + if not isinstance(value, list) or not value: + _fail( + f"{command} --segments must be a non-empty JSON array of " + f"{shape.summary} objects — got {_describe(value)}" + ) + for item in value: + _check_segment(item, shape, command) + return value + + +def _segments(args: argparse.Namespace) -> Optional[List[Dict[str, Any]]]: + """parse_segments() for whichever subcommand is running.""" + return parse_segments(args.segments, args.segments_shape, args.command) + + def build_client(api_key: Optional[str]) -> Sonilo: key = api_key or os.environ.get("SONILO_API_KEY") if not key: @@ -89,16 +221,20 @@ def cmd_text_to_music(client: Sonilo, args: argparse.Namespace) -> None: fmt = args.format use_async = args.use_async or fmt == "wav" out = _music_output(args, fmt) + segments = _segments(args) if use_async: result = client.text_to_music.generate_async( prompt=args.prompt, duration=args.duration, + segments=segments, output_format="wav" if fmt == "wav" else None, ) path = result.save(out) _wrote(path, path.stat().st_size) else: - track = client.text_to_music.generate(prompt=args.prompt, duration=args.duration) + track = client.text_to_music.generate( + prompt=args.prompt, duration=args.duration, segments=segments + ) path = track.save(out) _wrote(path, len(track.audio)) @@ -107,11 +243,13 @@ def cmd_video_to_music(client: Sonilo, args: argparse.Namespace) -> None: fmt = args.format use_async = args.use_async or fmt == "wav" or args.isolate_vocals or args.preserve_speech out = _music_output(args, fmt) + segments = _segments(args) if use_async: result = client.video_to_music.generate_async( video=args.video, video_url=args.video_url, prompt=args.prompt, + segments=segments, isolate_vocals=args.isolate_vocals or None, preserve_speech=args.preserve_speech or None, output_format="wav" if fmt == "wav" else None, @@ -120,7 +258,8 @@ def cmd_video_to_music(client: Sonilo, args: argparse.Namespace) -> None: _wrote(path, path.stat().st_size) else: track = client.video_to_music.generate( - video=args.video, video_url=args.video_url, prompt=args.prompt + video=args.video, video_url=args.video_url, prompt=args.prompt, + segments=segments, ) path = track.save(out) _wrote(path, len(track.audio)) @@ -142,7 +281,7 @@ def cmd_video_to_sfx(client: Sonilo, args: argparse.Namespace) -> None: out = args.output if args.output is not None else f"output.{args.format}" result = client.video_to_sfx.generate( video=args.video, video_url=args.video_url, - prompt=args.prompt, audio_format=args.format, + prompt=args.prompt, segments=_segments(args), audio_format=args.format, ) path = result.save(out) _wrote(path, path.stat().st_size) @@ -166,6 +305,7 @@ def _run_sound(client: Sonilo, args: argparse.Namespace, resource: Any, default_ video_url=args.video_url, music_prompt=args.music_prompt, sfx_prompt=args.sfx_prompt, + segments=_segments(args), preserve_speech=True if args.preserve_speech else None, ducking=False if args.no_ducking else None, ) @@ -269,6 +409,18 @@ def _add_video_source(parser: argparse.ArgumentParser) -> None: help="Remote video URL to score.") +def _add_segments(parser: argparse.ArgumentParser, shape: _SegmentShape) -> None: + parser.add_argument( + "--segments", default=None, + help=f"Timed segments, as a JSON array of {shape.summary} objects. " + "Pass the JSON inline, @FILE to read it from a file, " + "or @- to read it from stdin.", + ) + # Carried on the namespace so the one shared reader knows which contract + # to check the value against, and can name it when the check fails. + parser.set_defaults(segments_shape=shape) + + def build_parser() -> argparse.ArgumentParser: parser = _Parser(prog="sonilo", description="Command-line interface for the Sonilo API") parser.add_argument("--version", action="version", version=__version__) @@ -288,6 +440,7 @@ def build_parser() -> argparse.ArgumentParser: _add_global(p_t2m) p_t2m.add_argument("--prompt", required=True, help="What the music should sound like.") p_t2m.add_argument("--duration", type=int, required=True, help="Track length in seconds.") + _add_segments(p_t2m, MUSIC_SHAPE) p_t2m.add_argument("--output", default=None, help="Where to save the audio.") p_t2m.add_argument("--format", choices=["m4a", "wav"], default="m4a", help="Output container. wav forces async. Default: m4a") @@ -299,6 +452,7 @@ def build_parser() -> argparse.ArgumentParser: _add_global(p_v2m) _add_video_source(p_v2m) p_v2m.add_argument("--prompt", default=None, help="Optional creative direction.") + _add_segments(p_v2m, MUSIC_SHAPE) p_v2m.add_argument("--output", default=None, help="Where to save the audio.") p_v2m.add_argument("--format", choices=["m4a", "wav"], default="m4a", help="Output container. wav forces async.") @@ -323,6 +477,7 @@ def build_parser() -> argparse.ArgumentParser: _add_global(p_v2s) _add_video_source(p_v2s) p_v2s.add_argument("--prompt", default=None, help="Optional creative direction.") + _add_segments(p_v2s, SFX_SHAPE) p_v2s.add_argument("--output", default=None, help="Where to save the audio.") p_v2s.add_argument("--format", choices=_SFX_FORMATS, default="wav", help="Output format. Default: wav") @@ -337,6 +492,7 @@ def build_parser() -> argparse.ArgumentParser: help="Optional creative direction for the music bed.") p_v2sd.add_argument("--sfx-prompt", dest="sfx_prompt", default=None, help="Optional creative direction for the sound effects.") + _add_segments(p_v2sd, SFX_SHAPE) p_v2sd.add_argument("--preserve-speech", dest="preserve_speech", action="store_true", help="Keep source speech in the mix.") p_v2sd.add_argument("--no-ducking", dest="no_ducking", action="store_true", @@ -355,6 +511,7 @@ def build_parser() -> argparse.ArgumentParser: help="Optional creative direction for the music bed.") p_v2vsd.add_argument("--sfx-prompt", dest="sfx_prompt", default=None, help="Optional creative direction for the sound effects.") + _add_segments(p_v2vsd, SFX_SHAPE) p_v2vsd.add_argument("--preserve-speech", dest="preserve_speech", action="store_true", help="Keep source speech in the mix.") p_v2vsd.add_argument("--no-ducking", dest="no_ducking", action="store_true", diff --git a/sonilo-cli/tests/test_cli.py b/sonilo-cli/tests/test_cli.py index 18f9be7..c2b3f17 100644 --- a/sonilo-cli/tests/test_cli.py +++ b/sonilo-cli/tests/test_cli.py @@ -1,3 +1,4 @@ +import io import json from urllib.parse import unquote_plus @@ -536,3 +537,221 @@ def test_dubbing_without_languages_omits_the_field(tmp_path): # absent. Sending `languages=[]` or the string "None" would silently # override that default with something else. assert b"languages" not in route.calls.last.request.content + + +# --- --segments ----------------------------------------------------------- +# +# The two shapes are not interchangeable: music segments are +# {start, prompt, label?} and SFX segments are {start, end, prompt}. Both are +# sent as a JSON string inside the form-encoded body (see +# sonilo._requests.build_v2m_parts), so assertions decode the body first. + +MUSIC_SEGMENTS = [{"start": 0, "label": "intro", "prompt": "airy pads"}] +SFX_SEGMENTS = [{"start": 0, "end": 5, "prompt": "footsteps on gravel"}] + + +def _sent_segments(route): + """The `segments` value as it left the CLI, decoded back to Python.""" + body = unquote_plus(route.calls.last.request.content.decode()) + for field in body.split("&"): + name, _, value = field.partition("=") + if name == "segments": + return json.loads(value) + return None + + +@respx.mock +def test_segments_inline_json_reaches_the_request_body(tmp_path): + route = respx.post(f"{BASE}/v1/text-to-music").mock( + return_value=httpx.Response(200, text=_music_stream_body()) + ) + run(["text-to-music", "--prompt", "lofi", "--duration", "30", + "--segments", json.dumps(MUSIC_SEGMENTS), + "--output", str(tmp_path / "song.m4a")]) + assert _sent_segments(route) == MUSIC_SEGMENTS + + +@respx.mock +def test_segments_from_a_file(tmp_path): + route = respx.post(f"{BASE}/v1/text-to-music").mock( + return_value=httpx.Response(200, text=_music_stream_body()) + ) + src = tmp_path / "segments.json" + src.write_text(json.dumps(MUSIC_SEGMENTS)) + run(["text-to-music", "--prompt", "lofi", "--duration", "30", + "--segments", f"@{src}", "--output", str(tmp_path / "song.m4a")]) + assert _sent_segments(route) == MUSIC_SEGMENTS + + +@respx.mock +def test_segments_from_stdin(tmp_path, monkeypatch): + route = respx.post(f"{BASE}/v1/text-to-music").mock( + return_value=httpx.Response(200, text=_music_stream_body()) + ) + monkeypatch.setattr("sys.stdin", io.StringIO(json.dumps(MUSIC_SEGMENTS))) + run(["text-to-music", "--prompt", "lofi", "--duration", "30", + "--segments", "@-", "--output", str(tmp_path / "song.m4a")]) + assert _sent_segments(route) == MUSIC_SEGMENTS + + +@respx.mock +def test_sfx_segments_reach_the_request_body(tmp_path): + route = respx.post(f"{BASE}/v1/video-to-sound").mock( + return_value=httpx.Response(200, json={"task_id": "sg1", "status": "processing"}) + ) + respx.get(f"{BASE}/v1/tasks/sg1").mock( + return_value=httpx.Response(200, json=_sound_body("sg1")) + ) + respx.get("https://r2.example.com/sound.wav").mock( + return_value=httpx.Response(200, content=b"MIXED") + ) + run(["video-to-sound", "--video-url", "http://x/y.mp4", + "--segments", json.dumps(SFX_SEGMENTS), + "--output", str(tmp_path / "s.wav")]) + assert _sent_segments(route) == SFX_SEGMENTS + + +@respx.mock +def test_omitting_segments_sends_no_segments_field(tmp_path): + route = respx.post(f"{BASE}/v1/text-to-music").mock( + return_value=httpx.Response(200, text=_music_stream_body()) + ) + run(["text-to-music", "--prompt", "lofi", "--duration", "30", + "--output", str(tmp_path / "song.m4a")]) + # Not `segments=[]` and not the string "None" — the field must be absent, + # exactly as --languages is for dubbing. + assert b"segments" not in route.calls.last.request.content + + +def test_segments_unknown_keys_pass_through(tmp_path): + """A field the API adds later must not need a CLI release to be usable.""" + from sonilo_cli.__main__ import MUSIC_SHAPE, parse_segments + + value = [{"start": 0, "prompt": "pads", "intensity": 0.4}] + assert parse_segments(json.dumps(value), MUSIC_SHAPE, "text-to-music") == value + + +def test_segments_malformed_json_names_the_source(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", + "--segments", "[{start: 0}]"]) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert err.startswith("sonilo:") + assert "--segments" in err # the offending source + assert "Expecting" in err # the parser's own complaint + assert "Traceback" not in err + + +def test_segments_malformed_json_from_a_file_names_the_file(tmp_path, capsys): + src = tmp_path / "segments.json" + src.write_text("{oops") + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", f"@{src}"]) + assert exc.value.code == 1 + assert str(src) in capsys.readouterr().err + + +def test_segments_unreadable_file_exits_1(tmp_path, capsys): + missing = tmp_path / "nope.json" + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", f"@{missing}"]) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert str(missing) in err + assert "Traceback" not in err + + +def test_segments_must_be_a_list(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", + "--segments", '{"start": 0, "prompt": "pads"}']) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert "JSON array" in err + assert "{start, prompt, label?}" in err + + +def test_segments_must_not_be_empty(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", "[]"]) + assert exc.value.code == 1 + assert "empty array" in capsys.readouterr().err + + +def test_segments_elements_must_be_objects(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", '["intro"]']) + assert exc.value.code == 1 + assert "not an object" in capsys.readouterr().err + + +def test_music_command_rejects_sfx_shaped_segments(capsys): + """The predictable mistake, in the direction that is hardest to spot: the + music shape's required fields are all present, so only the foreign `end` + key gives it away.""" + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", + "--segments", json.dumps(SFX_SEGMENTS)]) + assert exc.value.code == 1 + err = capsys.readouterr().err + # Naming both the expected shape and the keys actually given is what makes + # this self-correcting without a docs lookup. + assert "video-to-music segments take {start, prompt, label?}" in err + assert "got an object with keys start, end, prompt" in err + + +def test_sfx_command_rejects_music_shaped_segments(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-sfx", "--video-url", "http://x/y.mp4", + "--segments", json.dumps(MUSIC_SEGMENTS)]) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert "video-to-sfx segments take {start, end, prompt}" in err + assert "got an object with keys start, label, prompt" in err + + +def test_segments_field_types_are_checked(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-sfx", "--video-url", "http://x/y.mp4", + "--segments", '[{"start": 0, "end": "5", "prompt": "thud"}]']) + assert exc.value.code == 1 + err = capsys.readouterr().err + assert "video-to-sfx segments take {start, end, prompt}" in err + assert "expected a number" in err + + +def test_segments_semantic_rules_are_left_to_the_server(): + """Spacing, the label enum, the first segment's start and item caps are + server-side rules; duplicating them here would drift. Shape-valid input + must pass client-side however implausible it looks.""" + from sonilo_cli.__main__ import MUSIC_SHAPE, parse_segments + + value = [{"start": 900, "prompt": "x", "label": "not-in-the-enum"}, + {"start": 901, "prompt": "y"}] + assert parse_segments(json.dumps(value), MUSIC_SHAPE, "text-to-music") == value + + +@pytest.mark.parametrize( + "command", + ["text-to-music", "video-to-music", "video-to-sfx", + "video-to-sound", "video-to-video-sound"], +) +def test_segments_help_shows_all_three_value_forms(command, capsys): + with pytest.raises(SystemExit): + main([command, "--help"]) + help_text = capsys.readouterr().out + assert "--segments" in help_text + assert "@FILE" in help_text + assert "@-" in help_text + + +@pytest.mark.parametrize("command", ["text-to-sfx"]) +def test_commands_without_segments_reject_the_flag(command, capsys): + """text-to-sfx takes no segments in the SDK, so the CLI must not offer it + (video-to-video-music, the other segment-less endpoint, has no CLI + command at all).""" + with pytest.raises(SystemExit) as exc: + run([command, "--prompt", "x", "--duration", "3", "--segments", "[]"]) + assert exc.value.code == 1 + assert "unrecognized arguments" in capsys.readouterr().err From fb97bd16a69285c9cb45df8cea149ba304e118b5 Mon Sep 17 00:00:00 2001 From: Spencer Qian Date: Fri, 24 Jul 2026 16:34:58 -0700 Subject: [PATCH 2/2] fix(cli): name the offending segment index in --segments errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns the Python messages with the Node CLI, which already pointed at the element that failed. A segments array is typically three or four items, so naming the index saves the reader counting: video-to-sfx segments take {start, end, prompt} — element 2 is not an object video-to-sfx segments take {start, end, prompt} — "prompt" must be a string (element 1) Indices are 0-based, matching Node. The shape mismatch message keeps its no-index form ("got an object with keys ...") in both CLIs, since that is a whole-payload mistake rather than one bad element. Also renames the stdin source in parse errors from "standard input" to "stdin", again matching Node. The non-empty-array error keeps naming the expected shape, which Node's does not — a deliberate difference, not an oversight. --- sonilo-cli/src/sonilo_cli/__main__.py | 16 ++++++++----- sonilo-cli/tests/test_cli.py | 34 +++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/sonilo-cli/src/sonilo_cli/__main__.py b/sonilo-cli/src/sonilo_cli/__main__.py index 2fd648f..0443b85 100644 --- a/sonilo-cli/src/sonilo_cli/__main__.py +++ b/sonilo-cli/src/sonilo_cli/__main__.py @@ -108,7 +108,7 @@ def _read_segments_source(raw: str) -> Tuple[str, str]: if not raw.startswith("@"): return raw, "--segments" if raw == _STDIN: - return sys.stdin.read(), "standard input" + return sys.stdin.read(), "stdin" path = raw[1:] if not path: _fail("--segments @ needs a filename, e.g. --segments @segments.json (@- reads stdin)") @@ -118,10 +118,14 @@ def _read_segments_source(raw: str) -> Tuple[str, str]: _fail(f"could not read segments from {path}: {exc.strerror or exc}") -def _check_segment(item: Any, shape: _SegmentShape, command: str) -> None: +def _check_segment(item: Any, index: int, shape: _SegmentShape, command: str) -> None: expected = f"{command} segments take {shape.summary}" + # Element-level problems name the offending index (0-based, matching the + # Node CLI): an array is usually three or four items, and pointing at one + # saves the reader counting. The shape mismatch below is the exception — + # it is a whole-payload mistake, so it reads better without an index. if not isinstance(item, dict): - _fail(f"{expected} — got {_describe(item)}, not an object") + _fail(f"{expected} — element {index} is not an object") # A key that belongs to the *other* shape is the tell for the one # predictable mistake here — SFX-shaped segments on a music command, or # the reverse — so it is reported instead of forwarded, even though it @@ -134,7 +138,7 @@ def _check_segment(item: Any, shape: _SegmentShape, command: str) -> None: _fail(f"{expected} — got {_describe(item)}") for field, kind in shape.required + shape.optional: if field in item and not _TYPE_CHECKS[kind](item[field]): - _fail(f"{expected} — got {field}={json.dumps(item[field])}, expected a {kind}") + _fail(f'{expected} — "{field}" must be a {kind} (element {index})') def parse_segments( @@ -158,8 +162,8 @@ def parse_segments( f"{command} --segments must be a non-empty JSON array of " f"{shape.summary} objects — got {_describe(value)}" ) - for item in value: - _check_segment(item, shape, command) + for index, item in enumerate(value): + _check_segment(item, index, shape, command) return value diff --git a/sonilo-cli/tests/test_cli.py b/sonilo-cli/tests/test_cli.py index c2b3f17..5d0de97 100644 --- a/sonilo-cli/tests/test_cli.py +++ b/sonilo-cli/tests/test_cli.py @@ -652,6 +652,15 @@ def test_segments_malformed_json_from_a_file_names_the_file(tmp_path, capsys): assert str(src) in capsys.readouterr().err +def test_segments_malformed_json_from_stdin_names_stdin(capsys, monkeypatch): + monkeypatch.setattr("sys.stdin", io.StringIO("{oops")) + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", "@-"]) + assert exc.value.code == 1 + # "stdin", not "standard input" — the Node CLI says stdin. + assert "could not parse segments from stdin:" in capsys.readouterr().err + + def test_segments_unreadable_file_exits_1(tmp_path, capsys): missing = tmp_path / "nope.json" with pytest.raises(SystemExit) as exc: @@ -683,7 +692,19 @@ def test_segments_elements_must_be_objects(capsys): with pytest.raises(SystemExit) as exc: run(["video-to-music", "--video-url", "http://x/y.mp4", "--segments", '["intro"]']) assert exc.value.code == 1 - assert "not an object" in capsys.readouterr().err + err = capsys.readouterr().err + assert "video-to-music segments take {start, prompt, label?}" in err + assert "element 0 is not an object" in err + + +def test_segments_element_errors_name_the_offending_index(capsys): + """Indices are 0-based, and point at the bad element rather than always + reporting the first — matching the Node CLI, so the two agree.""" + with pytest.raises(SystemExit) as exc: + run(["video-to-music", "--video-url", "http://x/y.mp4", + "--segments", '[{"start": 0, "prompt": "a"}, {"start": 5, "prompt": "b"}, "oops"]']) + assert exc.value.code == 1 + assert "element 2 is not an object" in capsys.readouterr().err def test_music_command_rejects_sfx_shaped_segments(capsys): @@ -718,7 +739,16 @@ def test_segments_field_types_are_checked(capsys): assert exc.value.code == 1 err = capsys.readouterr().err assert "video-to-sfx segments take {start, end, prompt}" in err - assert "expected a number" in err + assert '"end" must be a number (element 0)' in err + + +def test_segments_field_type_errors_name_the_offending_index(capsys): + with pytest.raises(SystemExit) as exc: + run(["video-to-sfx", "--video-url", "http://x/y.mp4", + "--segments", '[{"start": 0, "end": 5, "prompt": "a"},' + ' {"start": 5, "end": 9, "prompt": 7}]']) + assert exc.value.code == 1 + assert '"prompt" must be a string (element 1)' in capsys.readouterr().err def test_segments_semantic_rules_are_left_to_the_server():