From d179701ab820d3b984a2cdb21e83808b64ff4a3e Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 8 May 2026 07:27:39 +0000 Subject: [PATCH] ci(ui): bound the Recents-click step + structural data-testid selector The "Recents: click previous chat in sidebar" step in tests/studio/playwright_chat_ui.py was the single biggest wallclock sink across all three UI workflows on PR 5312: Linux Studio UI CI: 786s in this one step (out of 823s Drive chat UI) Windows Studio UI CI: 786s in this one step (out of 825s) Mac Studio UI CI: 1389s in this one step (out of 1542s) Root cause was the text-filtered selector aside a, aside button, [data-sidebar=sidebar] a, ... plus an EXCLUDE regex anchored start...end that didn't match the coalesced sidebar text the app actually renders (unslothBETA, UUnslothUnsloth, Train, Export, Recents). The loop kept clicking those nav links, the post-click page.evaluate threw on the navigated frame, the bare except: continue swallowed the error, and the loop iterated forward where each candidates.nth(i) hit Playwright's default 60s per-locator retry against a now-stale DOM. Mac under single-process Chromium ate about 22 of those retries. Server-side studio.log was idle for the entire 23-min window -- the time was spent in the browser. Fix: 1. Add data-testid=recent-thread to the actual chat-history SidebarMenuButton in studio/frontend/src/components/app-sidebar.tsx (the live one; thread-sidebar.tsx is dead code, no imports). Also add data-thread-type / data-thread-id for richer assertions. 2. Switch the Playwright selector to that testid, drop the text-match heuristic + EXCLUDE regex. 3. Bound the whole step with a 30s deadline + 5-iteration cap + 5s click timeout, so a misbehaving selector cannot blow up wallclock the way the previous loop did. Verified locally on Linux + headless Chromium: PASS: rendered 2 [data-testid=recent-thread] entries PASS: clicked recent inside deadline (about 0.6s used) PASS: bogus selector exits in 5s Test driver at tests/scripts/repro_recents_local.py. Expected savings on PR 5312: Linux UI 18m36s to about 5m Windows UI 24m47s to about 12m (still has about 7m install) Mac UI 31m10s to about 9m Total about 50 min compute and 22 min PR wallclock per PR. --- .../frontend/src/components/app-sidebar.tsx | 3 + tests/studio/playwright_chat_ui.py | 101 +++++++++--------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/studio/frontend/src/components/app-sidebar.tsx b/studio/frontend/src/components/app-sidebar.tsx index f00381b91d..278bb3fe64 100644 --- a/studio/frontend/src/components/app-sidebar.tsx +++ b/studio/frontend/src/components/app-sidebar.tsx @@ -527,6 +527,9 @@ export function AppSidebar() { {chatItems.map((item) => ( { diff --git a/tests/studio/playwright_chat_ui.py b/tests/studio/playwright_chat_ui.py index 57777c5cf5..23d3b7ea7d 100644 --- a/tests/studio/playwright_chat_ui.py +++ b/tests/studio/playwright_chat_ui.py @@ -964,64 +964,69 @@ with sync_playwright() as p: # thread-history loader / route param plumbing. # ───────────────────────────────────────────────────── step("Recents: click previous chat in sidebar") - # The sidebar lists threads as buttons / links. Match anything - # under the sidebar with non-empty text other than the nav - # entries we already verified (New Chat / Compare / Search / - # Recipes / Account). - EXCLUDE = re.compile( - r"^(New Chat|Compare|Search|Recipes|Settings|Account|" - r"Light Mode|Dark Mode|Developer|Help|Shutdown)$", - re.I, - ) - candidates = page.locator( - "aside a, aside button, [data-sidebar='sidebar'] a, " - "[data-sidebar='sidebar'] button" - ) - count_c = candidates.count() - clicked_recent = False # We sent the prompts ["Reply with exactly: hello", "What is 1+1?", # "Reply with exactly: world", ...] above. The thread title that # gets persisted is typically a snippet of the first user message # (Studio summarises after a few turns). We accept either a literal # word from one of our prompts OR a short Studio-summary heuristic. PROMPT_KEYWORDS = ("hello", "world", "tree", "yes", "1+1", "2+2") - for i in range(count_c): + # Use the structural data-testid the frontend renders on each + # chat-history entry (studio/frontend/src/features/chat/thread- + # sidebar.tsx). The previous text-filtered selector + # "aside a, aside button, [data-sidebar='sidebar'] a, ..." + # matched coalesced sidebar nav text like 'unslothBETA', + # 'UUnslothUnsloth' which the EXCLUDE regex didn't strip; the + # test then clicked nav links, lost its frame, hit per-locator + # timeouts and burned 13-23 minutes per platform on this single + # step (run 25537467494 macui = 23m9s, winui = 13m6s, linui = 13m5s). + # Belt-and-suspenders: bound the whole step at 30s so a misbehaving + # selector can never blow up wallclock the way the old loop did. + threads = page.locator('[data-testid="recent-thread"]') + deadline = time.monotonic() + 30 + clicked_recent = False + try: + threads.first.wait_for(state="visible", timeout=5_000) + except Exception as _wait_err: + info(f"WARN no recent-thread testid surfaced within 5s: {_wait_err!s}") + n_threads = threads.count() + for i in range(min(n_threads, 5)): + if time.monotonic() > deadline: + break try: - t = (candidates.nth(i).text_content() or "").strip() - if not t or EXCLUDE.match(t): - continue - # Heuristic: thread titles are typically the first - # user message snippet or a short summary. - if 3 <= len(t) <= 80: - candidates.nth(i).scroll_into_view_if_needed() - candidates.nth(i).click() - page.wait_for_timeout(1500) - shoot("15d-recent-clicked") - info(f"OK clicked recent entry: {t[:60]!r}") - clicked_recent = True - # Strict check: after clicking the Recents entry, the - # thread we land on must include at least one of our - # prompts in its rendered messages. Otherwise we - # navigated to a thread that wasn't ours. - turns_text = page.evaluate("""() => { - const els = document.querySelectorAll( - '[data-role="user"], [data-role="assistant"]' - ); - return Array.from(els).map(e => (e.innerText || '') - .toLowerCase()).join(' '); - }""") - if any(k in turns_text for k in PROMPT_KEYWORDS): - info("OK landed on a thread that includes our prompts") - else: - soft_fail( - "Recents-clicked thread doesn't contain any of our " - f"sent prompts; turns_text={turns_text[:120]!r}" - ) + t = (threads.nth(i).text_content() or "").strip() + threads.nth(i).scroll_into_view_if_needed() + threads.nth(i).click(timeout=5_000) + page.wait_for_timeout(500) + shoot("15d-recent-clicked") + info(f"OK clicked recent entry: {t[:60]!r}") + # Strict check: after clicking the Recents entry, the + # thread we land on must include at least one of our + # prompts in its rendered messages. + turns_text = page.evaluate("""() => { + const els = document.querySelectorAll( + '[data-role="user"], [data-role="assistant"]' + ); + return Array.from(els).map(e => (e.innerText || '') + .toLowerCase()).join(' '); + }""", None) + clicked_recent = True + if any(k in turns_text for k in PROMPT_KEYWORDS): + info("OK landed on a thread that includes our prompts") break - except Exception: + else: + soft_fail( + "Recents-clicked thread doesn't contain any of our " + f"sent prompts; turns_text={turns_text[:120]!r}" + ) + break + except Exception as _click_err: + info(f"recent-thread click {i} failed: {_click_err!s}") continue if not clicked_recent: - soft_fail("no Recents entry was clickable") + soft_fail( + f"no Recents entry was clickable within 30s deadline " + f"(n_threads={n_threads})" + ) # Back to chat. page.goto(f"{BASE}/chat") composer = page.locator('textarea[aria-label="Message input"]')