From 868e8c8160b2176b7c2381a329620bb18458c401 Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 11 Jul 2026 16:24:36 +0100 Subject: [PATCH 1/3] fix(video): fail fast when the duration tab probe misses an explicit --duration (#288) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _select_video_duration now raises UiSelectorDriftError (exit 23, #183 pattern) with a debug screenshot instead of warn-and-continue. The function only runs when the user passed --duration explicitly (the classic driver guards on request.duration is not None), so a probe miss means Flow's 8s default would silently replace the requested value — corrupting downstream timeline math (same contract principle as #281). --- .../api/transports/drivers/classic.py | 2 +- .../api/transports/ui_automation_video.py | 21 ++++++++++++++--- .../transports/test_ui_automation_video.py | 23 +++++++++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) 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..6b8cbf3a0 100644 --- a/src/gflow_cli/api/transports/ui_automation_video.py +++ b/src/gflow_cli/api/transports/ui_automation_video.py @@ -1096,11 +1096,16 @@ 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 + ) -> 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 (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 8s default corrupts + downstream timeline math.""" tab = await VideoGenerationMixin._probe_selector_cascade( page, "duration_tab", @@ -1108,7 +1113,17 @@ async def _select_video_duration(page: Page, seconds: int) -> None: ) 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 (8s) would " + f"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/test_ui_automation_video.py b/tests/api/transports/test_ui_automation_video.py index 6173fdf6c..f060122e7 100644 --- a/tests/api/transports/test_ui_automation_video.py +++ b/tests/api/transports/test_ui_automation_video.py @@ -349,9 +349,28 @@ async def test_clicks_the_duration_tab(self) -> 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) + msg = str(exc_info.value) + assert "4s" in msg + assert "--duration" in msg # remediation hint: omit --duration for Flow's default + + @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: From 02dcf011ef6105227ee3d1ca3e3ec10fed8fef4d Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 11 Jul 2026 16:37:20 +0100 Subject: [PATCH 2/3] =?UTF-8?q?fix(video):=20apply=20#289=20council=20find?= =?UTF-8?q?ings=20=E2=80=94=20driver=20test,=20docs,=20cleanups?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests/api/transports/drivers/test_classic.py: pin the classic-driver duration branch (explicit-duration guard, out_dir forwarding, drift error propagation) — the affected call site had no coverage (D4). - CHANGELOG [Unreleased] + KNOWN_ISSUES Open entry for the duration_tab probe miss, with the locale-label hypothesis for the selector rot (D5/D6). - Drop the pre-raise duration_not_set warning (old non-fatal event name firing on a fatal path — double signaling), drop the out_dir default to match sibling probe signatures, fix the 'aria-label' docstring (the selectors match visible tab text), soften the message to 'typically 8s' (D1/D2/D6). - Update the dev spike call site for the keyword-only out_dir. --- CHANGELOG.md | 4 ++ KNOWN_ISSUES.md | 17 +++++ scripts/dev/spike_movie_attach_payload.py | 2 +- .../api/transports/ui_automation_video.py | 13 ++-- tests/api/transports/drivers/test_classic.py | 68 +++++++++++++++++++ .../transports/test_ui_automation_video.py | 5 +- uv.lock | 2 +- 7 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 tests/api/transports/drivers/test_classic.py 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..65cbc4421 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -14,6 +14,23 @@ 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. **Workaround:** omit `--duration` to +accept Flow's default. Leading hypothesis: the tab label is rendered +locale-sensitively (e.g. "4 с" / "4 s") — the duration tabs match visible text +and have no locale-invariant fallback, unlike the aspect tabs' `crop_*` icon +ligatures. Investigation needs a headed browser against the live DOM; attach +the debug screenshot and your account locale to #288 if you hit this. + ### 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/ui_automation_video.py b/src/gflow_cli/api/transports/ui_automation_video.py index 6b8cbf3a0..33ad1efa1 100644 --- a/src/gflow_cli/api/transports/ui_automation_video.py +++ b/src/gflow_cli/api/transports/ui_automation_video.py @@ -1096,15 +1096,13 @@ async def _select_video_model( raise VideoModelSelectionError(detail=msg, route="model_option") @staticmethod - async def _select_video_duration( - page: Page, seconds: int, *, out_dir: Path | None = None - ) -> 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. 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 8s default corrupts + parameter — a silent fall-through to Flow's default corrupts downstream timeline math.""" tab = await VideoGenerationMixin._probe_selector_cascade( page, @@ -1112,14 +1110,13 @@ async def _select_video_duration( (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) 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 (8s) would " - f"silently replace the requested value (issue #288). Omit " + 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, ) 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 f060122e7..183a717ba 100644 --- a/tests/api/transports/test_ui_automation_video.py +++ b/tests/api/transports/test_ui_automation_video.py @@ -345,7 +345,7 @@ 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 @@ -357,10 +357,11 @@ async def test_missing_duration_tab_raises_drift_error(self) -> None: page = _cascade_page(set()) with pytest.raises(UiSelectorDriftError) as exc_info: - await VideoGenerationMixin._select_video_duration(page, 4) + 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: 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" }, From b3552fdf9832bceb353a25469a08c9d1cd57bf6d Mon Sep 17 00:00:00 2001 From: Flavio Oliva Date: Sat, 11 Jul 2026 18:00:55 +0100 Subject: [PATCH 3/3] docs(known-issues): duration control confirmed ABSENT from the cohort's settings popover (live e2e 2026-07-11) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live run on denon82: --duration 4 fail-fast verified (exit 23, screenshot, aborted pre-submit). Screenshot refutes the locale hypothesis — the panel renders aspect/count/model rows but no duration row at all. --- KNOWN_ISSUES.md | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/KNOWN_ISSUES.md b/KNOWN_ISSUES.md index 65cbc4421..8dbe57918 100644 --- a/KNOWN_ISSUES.md +++ b/KNOWN_ISSUES.md @@ -24,12 +24,20 @@ 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. **Workaround:** omit `--duration` to -accept Flow's default. Leading hypothesis: the tab label is rendered -locale-sensitively (e.g. "4 с" / "4 s") — the duration tabs match visible text -and have no locale-invariant fallback, unlike the aspect tabs' `crop_*` icon -ligatures. Investigation needs a headed browser against the live DOM; attach -the debug screenshot and your account locale to #288 if you hit this. +`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`