diff --git a/CHANGELOG.md b/CHANGELOG.md index 785db8ee1..a9900e099 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **An explicit `--duration` that cannot be applied now fails fast instead of silently producing a clip of Flow's default length.** When the video settings panel's duration tab probe missed (observed 3/3 on a live 2026-07-11 Frames-submode run: `--duration 4` returned an 8-second clip and the JSON result reported success), `_select_video_duration` demoted the failure to a warning and generation continued on Flow's default. Duration is a contract parameter — downstream timeline math sizes cuts from the requested value — so a probe miss with an explicit `--duration` now raises `UiSelectorDriftError` (**exit code 23**, the #183 selector-drift semantics) with a `debug_no_duration_tab.png` viewport screenshot and an omit-`--duration` remediation hint. Omitting `--duration` is unaffected. The underlying `duration_tab` selector-drift investigation stays open on #288 (#288, #289). + ## [0.31.0] — 2026-07-10 ### Fixed diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 3d159983e..8dbe57918 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -14,6 +14,31 @@ Living list of behaviour that's broken, surprising, or limited by design — alo ## Open +### Video duration tab probe misses on the current Frames-submode DOM + +- **Status:** Open ([#288](https://github.com/ffroliva/gflow-cli/issues/288)) +- **Severity:** Medium · **Affected:** `gflow video` with an explicit `--duration` + on at least some Classic-profile cohorts (3/3 miss on a live 2026-07-11 run) + +The `duration_tab` selector cascade (`[role='tab']:text-is('4s')` / +`:has-text('4s')`) fails to match on Flow's current Frames-submode settings +panel. Pre-fix this silently produced a clip of Flow's default length; since +the #289 fix the run fails fast with `UiSelectorDriftError` (exit 23) and a +`debug_no_duration_tab.png` screenshot — live-verified 2026-07-11 on the +denon82 profile (aborted pre-submit, saving the 10-credit generation). +**Workaround:** omit `--duration` to accept Flow's default. + +**Root cause (confirmed live 2026-07-11, screenshot evidence on #288): the +duration control is ABSENT from this cohort's settings popover** — the panel +renders mode tabs (Imagem/Video), sub-mode (Frames/Elementos), aspect +(9:16/16:9), count (1x–x4), and the model dropdown (Veo 3.1 - Lite), and +nothing else. The earlier locale hypothesis is refuted: the UI renders in +Portuguese and the sibling count tabs (`1x`/`x2`) match fine; there is simply +no duration row to click. Whether Flow removed clip-length selection for this +cohort/model or moved it elsewhere is unknown — until that's answered, +`--duration` cannot be honored on affected cohorts and the fail-fast is the +correct behavior, not a selector bug to patch. + ### macOS: generation runs logged-out → HTTP 401, even with `--browser chrome` - **Status:** **RESOLVED in v0.23.0** ([#222](https://github.com/ffroliva/gflow-cli/issues/222), diff --git a/scripts/dev/spike_movie_attach_payload.py b/scripts/dev/spike_movie_attach_payload.py index 6824ed4e5..4255dd0bd 100644 --- a/scripts/dev/spike_movie_attach_payload.py +++ b/scripts/dev/spike_movie_attach_payload.py @@ -106,7 +106,7 @@ async def _on_route(route: Any) -> None: if full: await VideoGenerationMixin._select_video_aspect(page, Aspect.PORTRAIT) # noqa: SLF001 await VideoGenerationMixin._set_output_count(page, 1) # noqa: SLF001 - await VideoGenerationMixin._select_video_duration(page, 8) # noqa: SLF001 + await VideoGenerationMixin._select_video_duration(page, 8, out_dir=None) # noqa: SLF001 await page.keyboard.press("Escape") await page.wait_for_timeout(600) await page.wait_for_timeout(800) diff --git a/src/gflow_cli/api/transports/drivers/classic.py b/src/gflow_cli/api/transports/drivers/classic.py index f7634f69b..954ef7d6d 100644 --- a/src/gflow_cli/api/transports/drivers/classic.py +++ b/src/gflow_cli/api/transports/drivers/classic.py @@ -156,7 +156,7 @@ async def configure_video_settings( ) if request.duration is not None: await VideoGenerationMixin._select_video_duration( # type: ignore[reportPrivateUsage] - page, request.duration + page, request.duration, out_dir=out_dir ) await page.keyboard.press("Escape") await page.wait_for_timeout(600) diff --git a/src/gflow_cli/api/transports/ui_automation_video.py b/src/gflow_cli/api/transports/ui_automation_video.py index 6f4554e83..33ad1efa1 100644 --- a/src/gflow_cli/api/transports/ui_automation_video.py +++ b/src/gflow_cli/api/transports/ui_automation_video.py @@ -1096,19 +1096,31 @@ async def _select_video_model( raise VideoModelSelectionError(detail=msg, route="model_option") @staticmethod - async def _select_video_duration(page: Page, seconds: int) -> None: + async def _select_video_duration(page: Page, seconds: int, *, out_dir: Path | None) -> None: """Click the duration tab for `seconds` (4/6/8, or 10 for omni_flash). - Disambiguated by aria-label text ('4s'..'10s'), NOT id-suffix + Disambiguated by visible tab text ('4s'..'10s'), NOT id-suffix (collides with count). Must run AFTER model select — the 10s tab only - exists once omni_flash is chosen. Non-fatal on miss.""" + exists once omni_flash is chosen. Fatal on miss (issue #288): this + only runs for an explicit --duration, and duration is a contract + parameter — a silent fall-through to Flow's default corrupts + downstream timeline math.""" tab = await VideoGenerationMixin._probe_selector_cascade( page, "duration_tab", (f"[role='tab']:text-is('{seconds}s')", f"[role='tab']:has-text('{seconds}s')"), ) if tab is None: - log.warning("ui_automation_video.duration_not_set", seconds=seconds) - return + shot = await _capture_debug_screenshot(page, out_dir, "debug_no_duration_tab.png") + raise UiSelectorDriftError( + selector_drift_detail( + "duration_tab", + f"the {seconds}s duration tab was not found on the Flow editor; " + f"refusing to proceed — Flow's default duration (typically 8s) " + f"would silently replace the requested value (issue #288). Omit " + f"--duration to accept Flow's default.", + shot, + ) + ) await tab.click() await page.wait_for_timeout(400) log.info("ui_automation_video.duration_set", seconds=seconds) diff --git a/tests/api/transports/drivers/test_classic.py b/tests/api/transports/drivers/test_classic.py new file mode 100644 index 000000000..9fba6cf06 --- /dev/null +++ b/tests/api/transports/drivers/test_classic.py @@ -0,0 +1,68 @@ +"""ClassicFlowUiDriver.configure_video_settings — duration branch (#288). + +The mixin unit tests pin `_select_video_duration` itself; these pin the +driver call site: the explicit-duration guard, the `out_dir` forwarding +(the screenshot is lost if a regression drops it), and that a probe-miss +`UiSelectorDriftError` propagates out of the driver instead of being +swallowed. +""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gflow_cli.api.transports.drivers.classic import ClassicFlowUiDriver +from gflow_cli.api.transports.ui_automation_video import VideoGenerationMixin +from gflow_cli.api.video import GenerateVideoRequest +from gflow_cli.errors import UiSelectorDriftError + + +def _settings_page() -> MagicMock: + page = MagicMock() + page.keyboard.press = AsyncMock() + page.wait_for_timeout = AsyncMock() + return page + + +def _stub_settings_helpers(monkeypatch: pytest.MonkeyPatch) -> AsyncMock: + """Stub every mixin helper configure_video_settings drives; return the + duration stub for assertions.""" + duration = AsyncMock() + monkeypatch.setattr(VideoGenerationMixin, "_select_video_model", AsyncMock()) + monkeypatch.setattr(VideoGenerationMixin, "_switch_video_sub_mode", AsyncMock()) + monkeypatch.setattr(VideoGenerationMixin, "_select_video_aspect", AsyncMock()) + monkeypatch.setattr(VideoGenerationMixin, "_set_output_count", AsyncMock()) + monkeypatch.setattr(VideoGenerationMixin, "_select_video_duration", duration) + return duration + + +class TestConfigureVideoSettingsDuration: + @pytest.mark.asyncio + async def test_explicit_duration_forwards_out_dir( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + duration = _stub_settings_helpers(monkeypatch) + page = _settings_page() + request = GenerateVideoRequest(prompt="a cat", duration=4) + await ClassicFlowUiDriver().configure_video_settings(page, request, out_dir=tmp_path) + duration.assert_awaited_once_with(page, 4, out_dir=tmp_path) + + @pytest.mark.asyncio + async def test_no_duration_skips_the_tab(self, monkeypatch: pytest.MonkeyPatch) -> None: + duration = _stub_settings_helpers(monkeypatch) + page = _settings_page() + request = GenerateVideoRequest(prompt="a cat") + await ClassicFlowUiDriver().configure_video_settings(page, request) + duration.assert_not_awaited() + + @pytest.mark.asyncio + async def test_drift_error_propagates(self, monkeypatch: pytest.MonkeyPatch) -> None: + duration = _stub_settings_helpers(monkeypatch) + duration.side_effect = UiSelectorDriftError(detail="probe=duration_tab: miss") + page = _settings_page() + request = GenerateVideoRequest(prompt="a cat", duration=4) + with pytest.raises(UiSelectorDriftError): + await ClassicFlowUiDriver().configure_video_settings(page, request) diff --git a/tests/api/transports/test_ui_automation_video.py b/tests/api/transports/test_ui_automation_video.py index 6173fdf6c..183a717ba 100644 --- a/tests/api/transports/test_ui_automation_video.py +++ b/tests/api/transports/test_ui_automation_video.py @@ -345,13 +345,33 @@ class TestSelectVideoDuration: async def test_clicks_the_duration_tab(self) -> None: sel = "[role='tab']:text-is('6s')" page = _cascade_page({sel}) - await VideoGenerationMixin._select_video_duration(page, 6) + await VideoGenerationMixin._select_video_duration(page, 6, out_dir=None) page.locator.assert_any_call(sel) @pytest.mark.asyncio - async def test_missing_duration_tab_is_non_fatal(self) -> None: + async def test_missing_duration_tab_raises_drift_error(self) -> None: + # #288: --duration is always explicit at this point (the classic driver + # guards on request.duration is not None) — a silent 4->8 substitution + # corrupts downstream timeline math, so a probe miss must fail fast. + from gflow_cli.errors import UiSelectorDriftError + + page = _cascade_page(set()) + with pytest.raises(UiSelectorDriftError) as exc_info: + await VideoGenerationMixin._select_video_duration(page, 4, out_dir=None) + msg = str(exc_info.value) + assert "4s" in msg + assert "--duration" in msg # remediation hint: omit --duration for Flow's default + assert "Screenshot:" not in msg # no out_dir -> no screenshot clause + + @pytest.mark.asyncio + async def test_missing_duration_tab_captures_screenshot(self, tmp_path: Path) -> None: + from gflow_cli.errors import UiSelectorDriftError + page = _cascade_page(set()) - await VideoGenerationMixin._select_video_duration(page, 10) # must not raise + with pytest.raises(UiSelectorDriftError) as exc_info: + await VideoGenerationMixin._select_video_duration(page, 6, out_dir=tmp_path) + assert "Screenshot:" in str(exc_info.value) + page.screenshot.assert_awaited() class TestSetOutputCount: diff --git a/uv.lock b/uv.lock index c892e7fa1..75287dd44 100644 --- a/uv.lock +++ b/uv.lock @@ -787,7 +787,7 @@ wheels = [ [[package]] name = "gflow-cli" -version = "0.30.0" +version = "0.31.0" source = { editable = "." } dependencies = [ { name = "browser-cookie3" },