From db3393fbc857af6e8fbcab643a95d212b81c8010 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 20 May 2026 02:20:15 -0700 Subject: [PATCH] studio/ci: harden three pre-existing CI flakes (#5627) * studio/ci: harden three pre-existing CI flakes Three independent fixes to flakes that have been failing on main for multiple PRs in a row and obscuring real signal. 1. tests/studio/playwright_chat_ui.py: The theme-toggle x3 block called acct.click() then waited 3s for [role="menu"] to appear. On slow CI runners the view-transition triggered by the previous cycle's theme toggle was still in flight when cycle 2 fired, the click landed during a Radix data-state= "closed" close-animation tick and silently no-oped. Symptom: "theme cycle 2: account menu didn't open" at line 963. Fix: (a) the "menu has detached" precondition now also treats data-state="closed" as gone; (b) timeout raised from 3s to 7s on the detach wait and 5s on the open wait; (c) one explicit click retry with an Escape press between attempts to drop any stray popup the first click might have toggled. 2. .github/workflows/consolidated-tests-ci.yml: unsloth_zoo @ main currently fails test_get_peft_model_passes_finetune_last_n_layers_through with "AttributeError: 'FakeModel' object has no attribute 'trainable_parameters'" -- unsloth_zoo/mlx/loader.py:2972 added a model.trainable_parameters() call that the test's fake model never stubbed. This blocks every unsloth PR's Core CI. Deselect the case alongside the existing two CUDA-only deselects until the loader fixture is fixed upstream. 3. .github/workflows/studio-{inference,mac-inference,windows-inference}-smoke.yml: The OpenAI/Anthropic multi-turn determinism check asserted strict string equality between two same-seed runs. llama-server can close the stream on a different batch-flush boundary across otherwise-identical greedy runs, varying a single trailing '\n' (run1: 'Paris.\n' vs run2: 'Paris.'). Generated tokens are the same; only trailing whitespace differs. Strip before comparing, keep the raw repr in the failure message so a real divergence stays diagnosable. * studio/ci: fall back to scroll + JS-click for theme menuitem PR #5627 fixed "account menu didn't open" but uncovered the next layer: on small macOS arm64 CI viewports the Radix dropdown can render the theme menuitem below the visible area, and force=True still requires in-viewport for click to land: Locator.click: Element is outside of the viewport - waiting for get_by_role("menuitem", ...).first - attempting click action - scrolling into view if needed - done scrolling The "done scrolling" line is misleading -- Playwright tries to scroll the element into the viewport but Radix's positioning math keeps it fixed off-screen, so the actionability gate fires. Three-tier click fallback: 1. force=True click with a 3s budget (current path). 2. scroll_into_view_if_needed() then click. 3. evaluate("el => el.click()") -- a synthetic DOM click that bypasses Playwright's viewport check entirely. Radix's menuitem handler only needs the click event, not a real pointer landing on a specific pixel. This is the same family of fix as the previous "treat data-state=closed as gone" patch: the test was assuming pointer-actionability semantics that the production menu component never required. --- .github/workflows/consolidated-tests-ci.yml | 10 ++- .github/workflows/studio-inference-smoke.yml | 9 +- .../workflows/studio-mac-inference-smoke.yml | 9 +- .../studio-windows-inference-smoke.yml | 9 +- tests/studio/playwright_chat_ui.py | 85 ++++++++++++++----- 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/.github/workflows/consolidated-tests-ci.yml b/.github/workflows/consolidated-tests-ci.yml index d0f60a8902..f8eccc7d18 100644 --- a/.github/workflows/consolidated-tests-ci.yml +++ b/.github/workflows/consolidated-tests-ci.yml @@ -356,11 +356,19 @@ jobs: # cases below auto-skip on a GPU-less runner; deselect them # explicitly so the no-CUDA outcome is "deselected", not "skipped", # making intent visible in the report. Env inherited from job block. + # + # test_get_peft_model_passes_finetune_last_n_layers_through is + # deselected because unsloth_zoo/mlx/loader.py at line 2972 calls + # model.trainable_parameters() on the fake-model fixture, which + # the test never stubbed; this fails on every platform regardless + # of CUDA. Tracked upstream as an unsloth_zoo bug; deselecting + # here unblocks unsloth CI until the loader fixture is fixed. working-directory: ${{ runner.temp }}/unsloth-zoo run: | python -m pytest -q --tb=short tests/ \ --deselect tests/test_unsloth_zoo_lora_merge.py::test_active_merge_device_returns_string_on_cuda_host \ - --deselect tests/test_unsloth_zoo_lora_merge.py::test_merge_lora_moves_cpu_inputs_to_active_device + --deselect tests/test_unsloth_zoo_lora_merge.py::test_merge_lora_moves_cpu_inputs_to_active_device \ + --deselect tests/test_mlx_finetune_last_n_layers.py::test_get_peft_model_passes_finetune_last_n_layers_through - name: unsloth_zoo — test_apply_fused_lm_head (lives in compiler.py) # `test_apply_fused_lm_head` lives at unsloth_zoo/compiler.py:1983, diff --git a/.github/workflows/studio-inference-smoke.yml b/.github/workflows/studio-inference-smoke.yml index 775363e73c..c533c4c896 100644 --- a/.github/workflows/studio-inference-smoke.yml +++ b/.github/workflows/studio-inference-smoke.yml @@ -259,7 +259,14 @@ jobs: for i, (a, b) in enumerate(zip(first, second), start = 1): print(f"[{label} turn {i}] {a!r}") assert a, f"{label}: empty turn {i} response" - assert a == b, ( + # Compare on stripped content: llama-server can vary + # trailing whitespace (specifically a final '\n') between + # otherwise-identical greedy runs depending on the + # batch-flush boundary at which the stream is closed. The + # generated tokens are identical; only the trailing + # whitespace differs. Keep the raw repr in the failure + # message so a real divergence is still legible. + assert a.strip() == b.strip(), ( f"{label} non-deterministic at turn {i} with temperature=0.0:\n" f" run1: {a!r}\n run2: {b!r}" ) diff --git a/.github/workflows/studio-mac-inference-smoke.yml b/.github/workflows/studio-mac-inference-smoke.yml index 2d6864e0cb..fab0a36bd1 100644 --- a/.github/workflows/studio-mac-inference-smoke.yml +++ b/.github/workflows/studio-mac-inference-smoke.yml @@ -263,7 +263,14 @@ jobs: for i, (a, b) in enumerate(zip(first, second), start = 1): print(f"[{label} turn {i}] {a!r}") assert a, f"{label}: empty turn {i} response" - assert a == b, ( + # Compare on stripped content: llama-server can vary + # trailing whitespace (specifically a final '\n') between + # otherwise-identical greedy runs depending on the + # batch-flush boundary at which the stream is closed. The + # generated tokens are identical; only the trailing + # whitespace differs. Keep the raw repr in the failure + # message so a real divergence is still legible. + assert a.strip() == b.strip(), ( f"{label} non-deterministic at turn {i} with temperature=0.0:\n" f" run1: {a!r}\n run2: {b!r}" ) diff --git a/.github/workflows/studio-windows-inference-smoke.yml b/.github/workflows/studio-windows-inference-smoke.yml index 2acc782984..ad739dd529 100644 --- a/.github/workflows/studio-windows-inference-smoke.yml +++ b/.github/workflows/studio-windows-inference-smoke.yml @@ -345,7 +345,14 @@ jobs: for i, (a, b) in enumerate(zip(first, second), start = 1): print(f"[{label} turn {i}] {a!r}") assert a, f"{label}: empty turn {i} response" - assert a == b, ( + # Compare on stripped content: llama-server can vary + # trailing whitespace (specifically a final '\n') between + # otherwise-identical greedy runs depending on the + # batch-flush boundary at which the stream is closed. The + # generated tokens are identical; only the trailing + # whitespace differs. Keep the raw repr in the failure + # message so a real divergence is still legible. + assert a.strip() == b.strip(), ( f"{label} non-deterministic at turn {i} with temperature=0.0:\n" f" run1: {a!r}\n run2: {b!r}" ) diff --git a/tests/studio/playwright_chat_ui.py b/tests/studio/playwright_chat_ui.py index 72c5f8e673..845451919c 100644 --- a/tests/studio/playwright_chat_ui.py +++ b/tests/studio/playwright_chat_ui.py @@ -939,27 +939,50 @@ with sync_playwright() as p: # Account-menu sets data-state="open" while the view- # transition is mid-flight; clicking it again before that # clears would no-op silently and the for-loop bailed - # after cycle 1 in earlier runs. + # after cycle 1 in earlier runs. The view transition triggered + # by the theme toggle can run >700ms on slow CI runners, so + # both the "menu detached" wait and the "menu appeared" wait + # need a comfortable budget; 3s was too tight and caused + # cycle-2 flake. try: page.wait_for_function( - """() => !document.querySelector('[role="menu"]')""", - timeout = 3_000, + """() => { + const m = document.querySelector('[role="menu"]'); + if (!m) return true; + // Radix sets data-state="closed" during the + // close animation; treat that as already gone. + return m.getAttribute('data-state') === 'closed'; + }""", + timeout = 7_000, ) except Exception: pass - page.wait_for_timeout(150) - try: - acct.click(force = True) - except Exception as exc: - soft_fail( - f"theme cycle {cycle + 1}: account-menu click failed " f"({exc!r})" - ) - break - # Wait for the dropdown menu to actually render before - # querying its items. - try: - page.wait_for_selector('[role="menu"]', timeout = 3_000) - except Exception: + page.wait_for_timeout(250) + # Try the click + wait; if the first click silently no-oped + # (e.g. mid-view-transition swallowed the event), retry once + # after pressing Escape to force-close any stray popup. + opened = False + for attempt in range(2): + try: + acct.click(force = True) + except Exception as exc: + if attempt == 1: + soft_fail( + f"theme cycle {cycle + 1}: account-menu click failed " + f"({exc!r})" + ) + continue + try: + page.wait_for_selector( + '[role="menu"][data-state="open"]', + timeout = 5_000, + ) + opened = True + break + except Exception: + page.keyboard.press("Escape") + page.wait_for_timeout(300) + if not opened: soft_fail(f"theme cycle {cycle + 1}: account menu didn't open") break theme_item = page.get_by_role( @@ -970,13 +993,35 @@ with sync_playwright() as p: page.keyboard.press("Escape") soft_fail(f"theme cycle {cycle + 1}: theme menuitem missing") break - try: - theme_item.click(force = True) - except Exception as exc: + # Click sequence with two fallbacks. On small CI viewports the + # Radix dropdown can render the theme item below the visible + # area; force=True still requires the element to be in the + # viewport, so the regular .click() fails with "Element is + # outside of the viewport". Fall back to scroll-into-view + + # click, then to a synthetic .click() via evaluate() that + # bypasses Playwright's viewport check entirely (Radix's + # menuitem handler only needs the click event, not a real + # pointer landing on a pixel). + click_err = None + for click_attempt in range(3): + try: + if click_attempt == 0: + theme_item.click(force = True, timeout = 3_000) + elif click_attempt == 1: + theme_item.scroll_into_view_if_needed(timeout = 2_000) + theme_item.click(force = True, timeout = 3_000) + else: + theme_item.evaluate("el => el.click()") + click_err = None + break + except Exception as exc: + click_err = exc + page.wait_for_timeout(200) + if click_err is not None: page.keyboard.press("Escape") soft_fail( f"theme cycle {cycle + 1}: theme menuitem click failed " - f"({exc!r})" + f"({click_err!r})" ) break # Settle. The ".dark" class on is the ground