Skip to content

test(chat): pause the page clock at a time the page has not passed - #767

Merged
devswha merged 1 commit into
mainfrom
fix/chat-size-pauseat-flake
Oct 11, 2026
Merged

devswha merged 1 commit into
mainfrom
fix/chat-size-pauseat-flake

Conversation

@devswha

@devswha devswha commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Fixes #766

scripts/chat-size-regression.ts called page.clock.install() and then pauseAt(new Date()) with a time taken in Node. The installed clock follows the page's own real time, which can already be past that Node time, and Playwright then throws pauseAt: Cannot fast-forward to the past. All four sites (the default keep-pane check, the phone check and two places in the "inactive attach lifecycle" scenarios) now go through pauseClock(page). It pauses at the page's Date.now() plus a 1 s margin for the round trip. No other script in scripts/ uses pauseAt.

Test-only change, so no CHANGELOG entry.

Verification

  • Before (main 5792d71): CHECK_MAX_RUNS=8 bun run check run --build /tmp/l9-loop.sh 16 … (runs bun scripts/chat-size-regression.ts 16 times on the check's isolated herdr): 12 pass / 4 fail. Every failure was pauseAt: Error: Cannot fast-forward to the past, in toggles, resume, writes and once outside the lifecycle loop.
  • After (this branch, same command): 16 pass / 0 fail.
  • bun run typecheck: pass.

pauseAt(new Date()) used a time taken in Node, which can be behind the page clock that clock.install() starts, and Playwright then throws 'Cannot fast-forward to the past'. Pause at the page's own time plus a margin instead.

Fixes #766
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e2716ca5-a8a3-46ff-b5b7-de9883b3b892

📥 Commits

Reviewing files that changed from the base of the PR and between 5792d71 and 47bf07c.


📒 Files selected for processing (1)
  • scripts/chat-size-regression.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.



📝 Summary

Summary by CodeRabbit

  • Tests
    • Improved consistency of automated checks for inactive chat attachments by aligning their simulated clock with the page’s current time. This helps avoid timing differences between the test environment and the page when verifying attach behavior. No user-facing functionality or behavior has changed.

Walkthrough

The chat-size regression script adds a helper that pauses the Playwright page clock at a time based on the page’s current time. Four inactive-attach checks now use the helper instead of pausing at Node’s current time.

Changes

Inactive-attach clock handling

Layer / File(s) Summary
Page-clock pause and scenario updates
scripts/chat-size-regression.ts
A new helper installs the page clock and pauses it 1,000 ms after the page’s current time. The keeping-window, pocket-phone, held-attach, and inactive-attach checks now use the helper.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: jiunshinn


Merge Risk: ⚪ Minimal · up to 47bf0

The test-only change anchors the regression checks to page time and addresses the reported intermittent clock-pause failures. No material merge-blocking risk is established; the PR is mergeable subject to normal checks.

Architecture Summary

Architecture risk: 🟡 Medium · up to 47bf0

The change affects 1 system.

Changed systems: scripts

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — scripts (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in scripts/chat-size-regression.ts: Added the local pauseClock(page) helper. It installs the page clock, reads the page’s current time, adds a 1,000 ms margin, and pauses at that time.
  • observed — Modified behavior in scripts/chat-size-regression.ts: The keeping-window scenario now pauses its clock through pauseClock instead of pausing at new Date().
  • observed — Modified behavior in scripts/chat-size-regression.ts: The pocket-phone scenario now pauses its clock through pauseClock instead of pausing at new Date().
  • observed — Modified behavior in scripts/chat-size-regression.ts: The held-attach scenario now pauses its clock through pauseClock instead of pausing at new Date().

Reliability and maintainability

  • inferred — Risk-relevant change factors for scripts: blast_radius_1; blast_radius_2; direct_dependents_1; direct_dependents_2

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the test change and explains that the page clock is paused at a valid time.
Description check Passed The description explains the failure, the resulting behavior, affected code paths, and validation results. It uses a "Verification" heading instead of the template's "Validation" heading, but it provi…
Linked Issues check Passed Issue [#766] requires removal of the race between the page clock and Node time in scripts/chat-size-regression.ts. The new pauseClock(page) installs the page clock, reads Date.now() in the page,…
Out of Scope Changes check Passed The diff only changes scripts/chat-size-regression.ts. It adds one helper and replaces the four clock-pause calls identified by issue [#766]. The helper comments and the reported typecheck support t…

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@devswha
devswha merged commit b576fb8 into main Oct 11, 2026
5 checks passed
@devswha
devswha deleted the fix/chat-size-pauseat-flake branch October 11, 2026 02:19
devswha added a commit that referenced this pull request Oct 11, 2026
## Change

Release 0.4.7, from main at 80ca9bb (47 PRs since v0.4.6).

- `package.json` and `herdr-plugin.toml`: 0.4.7.
- CHANGELOG: `[Unreleased]` becomes `[0.4.7] - 2026-10-12`, with the
compare links.
- #513's entry moves out of the 0.4.0 section, where its branch's merge
left it, and gets its credit; it says the pace is a setting, off by
default (#834).
- A Maintenance section covers the PRs that had no entry: #743, #756,
#767, #795, #805, #810, #813, #821, #826, #831.
- The note under the Hermes entry that asked for a runtime bump at
release now says what a remote PC needs.
- `release-summaries.json`: 0.4.7 in en/ko/ja/zh/tr (6 new, 6 improved,
5 fixed). The Turkish lines are not checked by a native speaker.
- `REMOTE_BUNDLE_VERSION` 25 -> 26. Since v0.4.5's runtime the bridge
gained Hermes transcripts (#604), conversation pushes (#689) and
Grok/Antigravity activity (#816), so `remote-v26` is published before
the app release.

Not in this release: #606.

## Validation

- `bun scripts/release-notes.ts 0.4.7` renders the summaries and the
full changelog (235 lines).
- `bun run typecheck` and `bun run build`: pass.
- `TMPDIR=/tmp bun run test:unit`: one failure, `server/voice.test.ts`
("audio.webm" vs "clip.webm"), which fails the same way on unmodified
`main` on this Mac (Bun 1.4.0); CI runs Bun 1.4.2.
- Every PR merged since v0.4.6 was checked against the changelog
section: all 47 are linked.

## Release

After this merges and its CI on `main` passes: push `remote-v26`, wait
for its release, then run the Release workflow with `0.4.7`.
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.

ci: chat-size-regression fails intermittently with pauseAt "Cannot fast-forward to the past"

1 participant