Skip to content

Fix the October flaky tests: sync delivery waits, PDF viewer swap invariant, typing hook, menu focus (bd-c72wsugj) - #818

Merged
cscheid merged 3 commits into
mainfrom
bugfix/bd-c72wsugj-flaky-ci-tests-pass
Oct 10, 2026
Merged

cscheid merged 3 commits into
mainfrom
bugfix/bd-c72wsugj-flaky-ci-tests-pass

Conversation

@cscheid

@cscheid cscheid commented Oct 10, 2026

Copy link
Copy Markdown
Member

Closes bd-c72wsugj (and its duplicates bd-fuw5gcni, bd-5sbguner, bd-tg3vnfxg, already closed in braid). Plan: claude-notes/plans/2026-10-10-flaky-ci-tests-pass.md.

What was flaking

  • doc-inventory.test.ts (local cargo xtask verify, filed four times): a flush race in the test helper. automerge-repo sends local changes through a 100 ms per-doc throttle; createNewProject awaits a SHA-256 between creating the text and the binary file, so under load the index doc's first flush carries only main.qmd. The helper's hubHasDoc presence check is satisfied by that partial index, and the creator then disconnects without draining. Reproduced 5 of 5 by delaying the digest.
  • pandoc-pdf-viewer "the preview pane" (WebKit, the red check on Listings: List.js init script, pagination and category filtering for default/grid listings #816 and two main runs): the swap gate accepted pdf.js's first draw at the default zoom, which happens before setInitialView applies the saved zoom and page and resets every page. Under load that draw finishes first, the new frame is promoted, and then empties itself. Both CI snapshots (page 7 selected at 150%, page 1 the only page with content; or all pages empty) are this.
  • pandoc-warm typing hook (WebKit): the hook slept one debounce after the last edit and checked for runs; the viewer iframe booting pdf.js on the same thread delayed the debounce past it.
  • projects-home-dialogs (Chromium): the New menu's placeholder item is replaced when the WASM registry answers; the focused placeholder unmounts, focus falls to <body>, and Enter goes nowhere. The spec had only ever exercised the placeholder.

Changes

quarto-sync-client. TestHub.hubHasHeads / hubHasHeadsOf check that the hub holds every change up to the client's heads (by change hash). Every project-creating test helper waits on that before the creator disconnects. project-creation-delivery.test.ts forces the slow-digest schedule; reverting the helpers to hubHasDoc fails it deterministically. The hub-client E2E project factory drains its disconnect and its "flush synchronously" comment is corrected.

PDF viewer. The promotion gate is now: pages loaded, initial view set, current page's view FINISHED, re-checked on documentinit and every pagerendered. The 5 s promote-anyway timeout is gone (decided: the frame on top is never an empty viewer); pdf.js's documenterror, or 30 s without a paint, make the swap fail and keep the previous document. PdfPreviewPane shows a failed swap in the same banner a failed compile gets, and its "a PDF is on screen" flag flips when the viewer has drawn rather than at hand-off. Unit tests drive a fake PDFViewerApplication through both draw orders, a document error, and the watchdog.

Typing hook. typing() waits for a run start after the last edit and for every run to end, instead of sleeping.

Menu. Menu keeps keyboard focus when the focused item is replaced (tracked by the root's focus events, refocused in a layout effect). The dialogs spec waits for the real menu and drives Templates → Default. The five pre-existing react-hooks/refs lint errors in Menu.tsx are fixed.

Docs. .claude/rules/hub-client-tests.md gains a "presence is not delivery" section and two Playwright bullets, and now also covers the sync-client and hub-mcp tests. Audit findings for the rest of the October harness batch are filed on bd-ag1q1eho.

Verification

  • cargo xtask verify --skip-hub-build: all steps pass.
  • quarto-sync-client: the touched suites plus exit-drain (26 tests) pass.
  • hub-client unit: pdfViewer (6), PdfPreviewPane (9), Menu.submenu (8), ProjectsHome.newMenu (4) pass; ESLint clean on touched files.
  • Harness, local, --retries=0 --workers=1: dialogs 8 of 8 under CPU load (a first version of the Menu fix failed 3 of 3 there, which is how the event-based version came about), typing hook 3 of 3 under load, viewer 6 of 6 at normal load and under load.
  • Honest gap: the old viewer gate's failure did not reproduce locally under CPU load or Chromium CPU throttling; that fix rests on the pdf.js source, the CI snapshots, and the unit test. CI on this PR is the real check.

🤖 Generated with Claude Code

cscheid and others added 3 commits October 10, 2026 09:31
… race; inventory the October harness flakes

Plan skeleton plus a deterministic repro. The binary-file entry goes
missing because createNewProject yields (await computeSHA256) between
its two index changes, automerge-repo's 100 ms trailing throttle splits
them across two flushes, hubHasDoc is a presence check satisfied by the
partial index, and the creator disconnects with drainMs 0 before the
second flush. Delaying crypto.subtle.digest by 150 ms reproduces the CI
diff on every run; a drained disconnect passes.

Also inventories the Playwright specs flaking in CI (PR #816 and main):
the WebKit pdf-viewer swap (two failure shapes), the warm typing hook,
and projects-home-dialogs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r swap gate, typing hook, menu focus

quarto-sync-client: presence is not delivery. TestHub gains hubHasHeads /
hubHasHeadsOf (the hub holds every change up to the client's heads, by
change hash); every project-creating test helper waits on that before the
creator disconnects, instead of hubHasDoc, which a partially flushed index
satisfies. project-creation-delivery.test.ts forces the slow-digest
schedule that lost logo.png's index entry under load. The hub-client E2E
project factory drains its disconnect for the same reason.

hub-client:
- pdfViewer promotes a recompile only once pages are loaded, the initial
  view is set and the current page's view is FINISHED; the first draw at
  the default zoom, which setInitialView resets, no longer counts. Unit
  test drives both orders through a fake PDFViewerApplication.
- typing() test hook waits for a run start after the last edit and for
  every run to end, instead of sleeping one debounce.
- Menu keeps keyboard focus when the focused item is replaced (the New
  menu's placeholder giving way to the registry's groups); the dialogs
  spec waits for the real menu and drives Templates > Default. The five
  react-hooks/refs lint errors in Menu.tsx are fixed on the way.

Rule .claude/rules/hub-client-tests.md gains the sync section and now
also covers the sync-client and hub-mcp tests. Audit findings for the
rest of the batch are filed on bd-ag1q1eho; the duplicate strands are
closed. Plan: claude-notes/plans/2026-10-10-flaky-ci-tests-pass.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ror (bd-c72wsugj)

The 5 s promote-anyway fallback is gone. A recompile's viewer is shown only
once it has painted the restored view, however long that takes; pdf.js's
documenterror, or 30 s without a paint (VIEWER_WATCHDOG_MS), make the swap
fail instead, and the previous document stays on top. PdfPreviewPane shows
a rejected swap in the same banner a failed compile gets, cleared by the
next compile, and its "a PDF is on screen" flag now flips when the viewer
has drawn rather than at hand-off. Unit tests cover the document error,
the watchdog, and the pane banner.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@posit-snyk-bot

posit-snyk-bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@cscheid

cscheid commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

@gordonwoodhull This PR changes some of the test assertions you wrote for the PDF viewer to fix some CI flakiness I noticed. I'm just letting you know in case I misunderstood the goal of the test and mangled its implementation.

@gordonwoodhull

Copy link
Copy Markdown
Member

Sounds good! We should probably have guidelines and best practices to generate less flaky tests, but I’ve just been fixing them on an ad-hoc basis so far.

@cscheid

cscheid commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

but I’ve just been fixing them on an ad-hoc basis so far.

Same here, just to clarify, and I agree that we should spend time sorting this out intentionally in the next few weeks or so.

@cscheid
cscheid merged commit 495a4fa into main Oct 10, 2026
13 of 14 checks passed
@cscheid
cscheid deleted the bugfix/bd-c72wsugj-flaky-ci-tests-pass branch October 10, 2026 22:28
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.

3 participants