test(desktop): remove the layout-tier E2E specs that block main by Astro-Han · Pull Request #4741 · apache/maka

Astro-Han · 2026-09-04T07:36:43Z

Summary

Five of the last six CI runs on main were red. Three of them were a Desktop E2E test sampling transcript geometry before it settled — a different test each time, in the same two files.

Four of the five open flake reports are in those two files: #4648, #4688 and #4707 in transcript-scroll.spec.ts, #4675 in prompt-rail.spec.ts. They are not five independent bugs. #4620 fixed this exact failure mode in transcript-scroll.spec.ts, and #4648 then reported it again elsewhere in the same file. Fixing them one at a time has not converged.

These 22 tests — a fifth of the suite — assert scroll offsets, bounding boxes and computed style against a compositor that settles on its own schedule. That needs a real layout engine, which Chromium provides; it does not need Electron. Under four-worker Xvfb load the suite reads a scroller mid-pin and blocks changes that never touched scrolling. retries: 0 is why this is visible at all and should stay; the tests are what is in the wrong place.

streaming-remount.spec.ts's interrupted-turn remount test (#4664) is removed for a different reason. It is not a timing artifact: the answer bubble mounts as historical while the Turn is live, which is a product race the test found and cannot itself fix. #4664 stays open as a product bug — the defect outlives its test.

This removes coverage without replacing it, which is the cost being paid deliberately to unblock main. Rebuilding it one tier down — Storybook + Chromium for geometry, act()-driven component tests for state machines — is the direction under discussion in #4727 and is not in this PR.

Also removed: promptRailMotionWindow, which had no other caller once prompt-rail.spec.ts was gone. The scrollMotion fixture option it fed keeps its unit coverage in scroll-motion-policy.test.ts and now has no E2E consumer; it is left in place for the rebuild rather than deleted and re-added.

Refs #4727. Closes #4648, #4675, #4688, #4707.

Verification

npm exec -w @maka/desktop -- playwright test --config e2e/playwright.config.ts --list — 88 tests in 35 files, down from 111 in 37. The delta is exactly the 23 removed.
npm run typecheck -w @maka/desktop — pass
npm run format — no fixes applied
npm run lint — no fixes applied
Not run: the full E2E suite. This PR's own CI run is the check that matters, and it exercises the remaining 88 against the same runner.

Review focus

Whether removing before rebuilding is the right order. The alternative — keep them red until the rebuild lands — leaves main blocked for as long as that takes.

AI use

Select exactly one:

No generative tool made a substantive contribution
Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code triaged the CI failures, identified which specs the open flake reports concentrate in, performed the removal, and traced what it left unreachable.

Checklist

Tests cover the change and fail without it — not applicable: this PR removes tests and adds none
Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

Yes — described under Summary above
No

Five of the last six CI runs on main were red, and three of them were one of these specs sampling transcript geometry before it settled. Four of the five open flake reports live in two files: #4648, #4688 and #4707 in transcript-scroll.spec.ts, #4675 in prompt-rail.spec.ts. Fixing them one at a time has not worked — #4620 fixed the same failure mode in the same file that #4648 then reported somewhere else in it. These 22 tests need a real layout engine, not Electron. They assert scroll offsets and bounding boxes against a compositor that settles on its own schedule, so under four-worker Xvfb load they read a scroller mid-pin and block changes that never touched scrolling. retries: 0 is why this is visible and stays that way; the tests are what is in the wrong place. streaming-remount.spec.ts's interrupted-turn remount test (#4664) goes with them for a different reason: it is not a timing artifact. The bubble mounts as historical while the Turn is live, which is a product race the test found and cannot itself fix. #4664 stays open as a product bug. Removes what that leaves unreachable: promptRailMotionWindow had no other caller. The scrollMotion fixture option below it keeps its unit coverage and no E2E consumer until this coverage is rebuilt one tier down. Refs #4727 Generated-by: Claude Code

M4n5ter

LGTM

…on drag `a one-line Markdown code block exposes native and selection horizontal scrolling` is the file's only test, and it asserts six geometry values against a real scroller. It failed this PR's own first CI run with `Expected: > 0` / `Received: 0` on the selection drag — the flake already reported against it, on a branch that changes nothing it touches. Same tier and same reason as the specs #4741 removes: a real layout engine settles this on its own schedule, and Electron adds nothing the claim needs. It is rebuilt one tier down with the rest, per #4727. Generated-by: Claude Code

Three defects in the desktop test suite, none of them a product fault, all of them blocking main or this PR. The workspace suite ran to its 900s ceiling. `renderer-facing Runtime Host protocol does not load Node crypto` pointed a Vite dev server at the real apps/desktop/src/renderer, and workspacePackagesPlugin(repoRoot) pushed the repository manifest plus every workspace manifest into configFileDependencies. The watcher following them walked the whole repository; server.close() did not wait for that walk, and the unfinished fs requests held the process open after both tests had passed. Measured with process.getActiveResourcesInfo() right after close(): FSReqPromise and FSReqCallback survive with the watcher and do not without it — esbuild.stop() changes nothing, and pgrep -P shows no child processes. That test sends one request and makes one assertion, so it gets `watch: null`. The first test keeps its watcher: re-resolving after a manifest edit is what it asserts, and its temporary-directory root is why it never hung. 900s becomes 0.29s. Failing on main since #4705 landed — d422246 and again on an unrelated PR. The same test passed on a document that was not the module. Without packages/runtime-host/dist, Vite answers the @fs/ request with the SPA fallback: 200, and index.html carries no vite-browser-external:node:crypto, so the assertion passed on index.html. CI always builds first and never saw it; any unbuilt tree got a green test that checked nothing. The fallback is now rejected explicitly, naming the build that is missing. code-scroll.spec.ts flaked this PR's own first run — `Expected: > 0` / `Received: 0` on its selection drag, on a branch that changes nothing it touches. It is the file's only test, it asserts six geometry values against a real scroller, and it is the same tier and failure mode as the specs #4741 removed. It is removed with them; the rebuild is one tier down, per #4727. Refs #4705 Refs #4741 Refs #4727 Generated-by: Claude Code

Add separate zh-CN and zh-TW resolved locales, including Taiwan-localized UI catalogs and system-locale aliases, while preserving the former persisted zh preference as zh-CN. Replace the Desktop Settings language segmented control with the @maka/ui Selector, offering Follow system, Simplified Chinese, Traditional Chinese, and English. Rebased onto latest main; resolved conflicts with the models settings redesign (apache#4682), the RuntimeInvocation event spine (apache#4631), the intentional Runtime Host quit (apache#4709), and the removed layout-tier E2E specs (apache#4741). Co-authored-by: AcerYue <33564291+AcerYue0@users.noreply.github.com> Co-authored-by: JLee <pkh_aceryue@photons.com.tw>

The Desktop composer withholds `/compact` while a Turn streams. #4741 deleted the only case that covered it, and the surviving slash-command spec opens the menu after the Turn ends, where `streaming` is false — so nothing catches a regression today. The rule lived inside the array filter that builds the menu, reachable only by rendering AppShell. It moves to `desktop-slash-command.ts`, next to the parser that owns the same four commands, and `node:test` asserts both halves: an idle Session is offered `/compact`, a streaming one is offered everything else and nothing less. Dropping the guard fails that test. The catalog query stays in AppShell on purpose: the debt ratchet counts dependencies per file, and moving `@maka/core/slash-command-catalog` into the smaller module would book a new dependency there rather than retire one. For the same reason the unused `enqueueInteraction` import goes — the predicate needs an import specifier, and AppShell had a dead one to spend. Refs #4727 #4752 Generated-by: Claude Code

Add separate zh-CN and zh-TW resolved locales, including Taiwan-localized UI catalogs and system-locale aliases, while preserving the former persisted zh preference as zh-CN. Replace the Desktop Settings language segmented control with the @maka/ui Selector, offering Follow system, Simplified Chinese, Traditional Chinese, and English. Rebased onto latest main; resolved conflicts with the models settings redesign (apache#4682), the RuntimeInvocation event spine (apache#4631), the intentional Runtime Host quit (apache#4709), and the removed layout-tier E2E specs (apache#4741). Co-authored-by: AcerYue <33564291+AcerYue0@users.noreply.github.com> Co-authored-by: JLee <pkh_aceryue@photons.com.tw>

#4741 removed prompt-rail.spec.ts along with transcript-scroll.spec.ts, and for the same reason. This finishes #4761's P0 by putting its assertions in the same tier as the previous two commits: ten of the eleven, one waiting on shell state. That spec existed because the rail failed three times in a row the same way — the code kept working and the pixels stopped. #2161 pinned it against .maka-chat-shell while Astryx's ChatLayout owned the scroll container, so it laid out across the whole conversation and scrolled off screen. #2338 parked it under macOS's overlay scrollbar, which takes no layout space but still swallows the pointer, so every tick rendered and none could be clicked. #2580 moved the tick onto Astryx's Button, whose label span put the bar back into normal flow, and an inline box takes no width or height, so the bars computed to 0x0 and shipped invisible in 0.1.9 and 0.1.10. None of the three is visible to a static read of the CSS and none is reachable from jsdom. All three need a real scroller with a real transcript. None needs Electron. The 120-prompt seeded session turned out not to be needed either. ChatView reads two props: the transcript carries only the Host's bounded active range, and transcriptTurnIndex carries the remaining landmarks. So the rail gets its full 64 ticks against 10 mounted Turns, which is what production does. A tick for a Turn outside the range comes back out as onLoadTranscriptTurn, so the jump that used to look dead — the head not mounted, the fill changing scrollHeight under the tail-follow lock — is reachable by moving the range in the harness. Two things a green run here does not mean. PromptRailTickOwnsItsOwnHitBox guards #2338, and it is load-bearing on macOS only: Linux's in-flow scrollbar moves the content column left instead of overlaying it, so the regression goes green on CI. That was already true in E2E. The comment says to run it locally on macOS before touching the rail's right edge. RailStaysOnTheVisiblePrompt no longer walks all 120 prompts of history. It asserts at five reading positions plus one jump that replaces the active range, keeping both original assertions — exactly one current tick, and it maps from the Turn being read — with a MutationObserver watching the count across every change rather than sampling at rest. Verified by mutation: offsetting the expected tick index by one fails it. Switching Sessions and rebuilding only the Host active range is not here. It needs shell state app-shell.tsx holds (#4582). Refs #4761. Generated-by: Claude Code

Add separate zh-CN and zh-TW resolved locales, including Taiwan-localized UI catalogs and system-locale aliases, while preserving the former persisted zh preference as zh-CN. Replace the Desktop Settings language segmented control with the @maka/ui Selector, offering Follow system, Simplified Chinese, Traditional Chinese, and English. Rebased onto latest main; resolved conflicts with the models settings redesign (apache#4682), the RuntimeInvocation event spine (apache#4631), the intentional Runtime Host quit (apache#4709), and the removed layout-tier E2E specs (apache#4741). Co-authored-by: AcerYue <33564291+AcerYue0@users.noreply.github.com> Co-authored-by: JLee <pkh_aceryue@photons.com.tw>

…#4766) #4741 deleted `transcript-scroll.spec.ts` (774 lines) and `prompt-rail.spec.ts` (622) because they asserted scroll offsets and bounding boxes against a compositor settling on its own schedule, and blocked `main` while doing it. That was the right call for the required check, and it left transcript scrolling and the prompt rail with no automated coverage at all — including the three regressions the rail spec existed for, each of which shipped: the rail laid out across the whole conversation and scrolled off screen (#2161), parked under macOS's overlay scrollbar where every tick rendered and none could be clicked (#2338), and computed to 0x0 bars, invisible in 0.1.9 and 0.1.10 (#2580). The instability was never in the readings. It was in asserting them under four Xvfb workers sharing OS focus and throttled frames. These assertions need a real layout engine; none of them needs Electron. Storybook renders in the same Chromium, settles per story rather than per application launch, and has nothing competing for focus. Rebuilds 18 of the 22 deleted tests as Storybook `play` functions, in eighteen stories, plus one that is new. No production code changes. Nothing had to be extracted first: `ChatView` already takes the transcript, the history seam, the growth signal and the rail's landmark index as props, so a story reaches every one without a fake backend. The rail's 120-prompt seeded session was not needed either — the transcript carries the Host's bounded active range and `transcriptTurnIndex` carries the rest of the landmarks, so the rail gets its full 64 ticks against 10 mounted Turns, which is what production does. The nineteenth story is not a restoration. Every deleted test watches content arriving at a reader who stays put; none watches a reader travelling *up* through `content-visibility` placeholders as they materialise, which is where #4259 measured one traversal moving `scrollHeight` by 63%. Measuring first changed what it could assert: the traversal is not still and cannot be, since a Turn off screen is laid out at its `contain-intrinsic-block-size` estimate and swaps to its real height on the way past. So the story bounds it — no single step past a whole Turn, the whole traversal within 15%, and the reader can still dock back. That bound is the property #4206 bought: one estimate to correct per Turn, so the correction scales with Turns crossed rather than with what is inside them. Four of the deleted tests are not rebuilt, for three different reasons: - *a gesture a nested scroller consumed does not release the tail* — a tier decision. It turns on Chromium's own scroll chaining, which needs real wheel input. Stays in E2E. The sibling case sinks because its guard reads `composedPath()` and the overflow of what the wheel crossed, which is DOM state a dispatched wheel reaches identically. - *streaming deltas do not reconstruct the prompt rail observer* — uncovered until it lands as a component test (#4761). It asserts no geometry, so a layout engine buys it nothing. - *switching Sessions restores a Turn anchor* and *switching sessions reconstructs only the Host active range* — uncovered, waiting on #4582. Not a tier decision: they need shell state `app-shell.tsx` holds, not a real window. Three ported assertions are not byte-for-byte, and the description lists them rather than leaving them to be discovered: the accessibility-tree half of the offscreen-Turn test needs CDP and did not come across; the nested-scroller test swapped its closing tail-follow check for a direct assertion that no history was requested; and the rail-currency test asserts at five reading positions plus a range-replacing jump with a MutationObserver, rather than walking all 120 prompts. Twelve of the stories were mutation-tested against the real defect they name — `display: inline` on the tick bar, `position: static` on the rail anchor, both directions of the tail-follow ResizeObserver branch, dropping the wheel guard, `contain-intrinsic-block-size: 60px`, `content-visibility: hidden` — and go red for the right reason. One story was found genuinely flaky under review (4 failures in ~310 runs at 4x CPU throttle) because it read its anchor outside any retry, on a layout the arriving Turn had not finished; fixed and re-checked at 0 in 40 runs at 4x and 60 at 8x, still red under mutation. One change outside the stories: `SMOKE_HEADED=1` on the visual smoke runner. The #2338 guard is inert headless — headless Chromium paints no platform scrollbar, and Linux's in-flow one moves the content column left instead of overlaying it — so "run it locally on macOS" was an instruction that bought nothing until there was a way to run it headed. No migration or compatibility impact. Storybook gains eighteen stories that exist to be asserted against rather than looked at. Refs #4761. Generated-by: Claude Code

github-actions Bot added the effort/XL Under 2500 readable lines label Sep 4, 2026

orangeCatDeveloper approved these changes Sep 4, 2026

View reviewed changes

M4n5ter approved these changes Sep 4, 2026

View reviewed changes

Astro-Han marked this pull request as ready for review September 4, 2026 08:23

Astro-Han merged commit 376a06f into main Sep 4, 2026
3 of 4 checks passed

Astro-Han deleted the test/desktop-e2e-remove-flaky-scroll-rail branch September 4, 2026 08:23

Astro-Han mentioned this pull request Sep 4, 2026

fix(desktop): unblock the test job from its own tests #4748

Merged

6 tasks

testikun mentioned this pull request Sep 4, 2026

refactor(desktop): register Workbar surface descriptors #4747

Open

6 tasks

Astro-Han mentioned this pull request Sep 4, 2026

test(desktop): sink the slash-command submit rule to its routing function #4752

Merged

6 tasks

Astro-Han mentioned this pull request Sep 4, 2026

test(desktop): rebuild transcript and prompt rail coverage as stories #4766

Merged

6 tasks

Navigation Menu

Uh oh!

test(desktop): remove the layout-tier E2E specs that block main - #4741

test(desktop): remove the layout-tier E2E specs that block main#4741
Astro-Han merged 1 commit into
mainfrom
test/desktop-e2e-remove-flaky-scroll-rail

Astro-Han commented Sep 4, 2026

Uh oh!

M4n5ter left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Astro-Han commented Sep 4, 2026

Summary

Verification

Review focus

AI use

Checklist

Uh oh!

M4n5ter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants