Skip to content

fix: clear the CI reds on main (Mermaid E2E sampling, plan-reminder autofocus, harbor-cell deadline race) - #2001

Merged
Astro-Han merged 3 commits into
mainfrom
test/2000-mermaid-toolbar-offset
Aug 3, 2026
Merged

fix: clear the CI reds on main (Mermaid E2E sampling, plan-reminder autofocus, harbor-cell deadline race)#2001
Astro-Han merged 3 commits into
mainfrom
test/2000-mermaid-toolbar-offset

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Three independent failures were keeping main's CI red. Each is fixed at its cause, not by widening a tolerance or adding a retry.

1. Mermaid toolbar offset E2E (e2e_shard 2/2) — the assertion measured the wrong thing. Failing on six consecutive runs across unrelated branches plus two on main, reading 2px or 3px against a <= 1 bound. The offset it measures cannot move: Toolbar is the first flex child of figure.maka-mermaid-diagram, and the figure carries a 1px border, so toolbar.top - figure.top is structurally that border regardless of zoom. The change came from the sampling — the offset was derived from two independent boundingBox() round-trips, and the transcript is a bottom-pinned scroller that re-pins on every ResizeObserver update (use-chat-scroll.ts). Zooming widens the canvas, the viewport gains a scrollbar, mermaid-diagram.tsx recomputes viewportHeight on the next frame, the figure changes height, and the scroller re-pins — landing between the two samples and turning scroll drift into a phantom offset. Both the toolbar offset and the viewport height are now read inside one evaluate, and the comparison polls for the steady state instead of the frame right after the click. The 1px tolerance is unchanged.

2. Plan-reminder edit dialog (e2e_shard 1/2) — a real product regression. #2002 rebuilt the dialog's title row as a bare <input autoFocus> inside a Field, replacing a TextInput with hasAutoFocus. Astryx's Dialog picks its initial focus target by querying [data-autofocus] after showModal(), and its own source documents why React's autoFocus cannot work there: it calls .focus() during commit, while the dialog is still invisible, so the focus silently fails. TextInput emits data-autofocus; the bare input did not, so opening 编辑提醒 left the caret on the close button instead of the title field. Moved onto the same seam. The E2E was reporting a genuine bug.

3. Harbor-cell force-stop test (test_headless) — a race, not a flake. The test asserts the deadline stops an active isolated tool, but established that precondition by giving the cell 1000ms of wall clock to finish setup, create the session, and drive the first send before the timer fired. Lose that race and the run is still cancelled by benchmark.deadlinesettledByDeadline stays true — while the backend is never stopped, so stopModes is empty. Setup costs ~100ms locally; on a loaded runner it can exceed the budget. The setup budget is now 3000ms and the precondition is asserted directly, so losing the race reports "the isolated tool never started within the budget" instead of a bare deepEqual mismatch.

Closes #2000

Root cause evidence

Verification

  • Full CI green on this branch (run 30811370682): both E2E shards, test_headless, test_workspaces, typecheck, alignment_audit.
  • Full headless suite locally: 1305 tests, 0 fail.
  • The new headless guard was exercised in both directions — passes normally, and fails with its intended message when the race is forced.
  • @maka/ui typecheck and Biome clean on the touched files.
  • E2E could not be run locally: this environment has no window server, so Electron launches but firstWindow() times out. CI is the verification for Add Rive workflow Maka tool #1 and Harden runtime, storage, gateway, credentials, and IPC inputs #2.

`toolbar.top - figure.top` is structurally the figure's 1px top border:
the Toolbar is the figure's first flex child, so zoom cannot move it. The
assertion still read 2-3px of change in CI because it derived that offset
from two independent boundingBox() round-trips, and the transcript is a
bottom-pinned scroller that re-pins on every ResizeObserver update — the
zoom-induced viewport reflow lands between the two samples and the scroll
drift shows up as a phantom offset.

Read both the toolbar offset and the viewport height inside one evaluate,
so the pair is sampled in a single frame, and poll for the steady state
instead of the frame right after the click: the zoomed layout settles over
a rAF, a ResizeObserver pass, and the scroller's re-pin. The 1px tolerance
is unchanged; only the sampling is fixed.

Closes #2000
@Astro-Han
Astro-Han force-pushed the test/2000-mermaid-toolbar-offset branch from 5ee7c27 to 36eeac1 Compare August 3, 2026 11:41
#2002 rebuilt the dialog's title row as a bare <input autoFocus> inside a
Field, replacing a TextInput with `hasAutoFocus`. Astryx's Dialog picks its
initial focus target by querying `[data-autofocus]` after showModal(), and
its own source documents why React's autoFocus cannot work here: it calls
.focus() during commit, while the dialog is still invisible, so the focus
silently fails. TextInput emits `data-autofocus`; the bare input did not,
so opening 编辑提醒 left the caret on the close button instead of the title.

Move the field onto the same seam. Caught by the E2E that asserts the title
box is focused when the edit dialog opens.
The force-stop test asserts that the deadline stops an *active* isolated
tool, but established that precondition by giving the cell 1000ms of wall
clock to finish setup, create the session, and drive the first send before
the timer fired. Lose that race on a loaded runner and the run is still
cancelled by `benchmark.deadline` — settledByDeadline stays true — while
the backend is never stopped, so `stopModes` is empty. That is the CI
failure: actual [] vs expected ['immediate'].

Reproduced by forcing the ordering with a 1ms deadline, which yields the
same signature. Budget the setup at 3000ms instead of 1000ms and assert
the precondition directly, so losing the race reports 'the isolated tool
never started within the budget' rather than a bare deepEqual mismatch.
@Astro-Han Astro-Han changed the title test(e2e): sample Mermaid toolbar geometry atomically fix: clear the CI reds on main (Mermaid E2E sampling, plan-reminder autofocus, harbor-cell deadline race) Aug 3, 2026
@Astro-Han
Astro-Han marked this pull request as ready for review August 3, 2026 11:58
@Astro-Han
Astro-Han merged commit f7cb2c3 into main Aug 3, 2026
11 checks passed
Astro-Han pushed a commit that referenced this pull request Aug 3, 2026
Squashed onto latest main. The PR's own CI was red for a reason unrelated to its code: the branch snapshot predated #2001's plan-reminder autofocus fix (f7cb2c3), so e2e_shard (1/2) failed on plan-reminders.spec.ts:86 (toBeFocused) under the stale merge-ref. Main's e2e has been green since #2001; squashing re-bases the change onto the fixed code.

Independent review (3x deepseek-v4-flash): PASS, no P0/P1. Retry/deadline/fail-closed mechanism verified (RED: database is locked on old logic; GREEN: 0 failures; non-BUSY errors throw immediately; 5s bound enforced).

Known follow-up (pre-existing, not blocking): the same fresh-open WAL race remains in configureSqliteSessionMetadataDatabase (no retry on journal_mode switch); track separately if desired.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(e2e): stabilize Mermaid toolbar offset assertion

1 participant