fix: prevent corrupted seams during scrolling auto-scroll - #538
Draft
BenjaminD2023 wants to merge 6 commits into
Draft
fix: prevent corrupted seams during scrolling auto-scroll#538BenjaminD2023 wants to merge 6 commits into
BenjaminD2023 wants to merge 6 commits into
Conversation
Auto Scroll now posts one bounded wheel burst, waits for frames captured after the last synthetic event, then requests a single stitch commit. Synthetic events no longer drive the manual commit loop, and a known step prior is not replaced by the last accepted delta. The stitcher rejects matches that strongly contradict the current expected step so skipped and duplicated seams cannot chain.
Add closed-loop Auto Scroll controller tests, known-step stitcher regressions for skipped and duplicated bands, and accuracy-benchmark cases for intermediate frames, skipped content, duplicates, and a final small boundary step.
Describe bounded Auto Scroll steps, settle-then-commit sequencing, the split between manual and Auto Scroll scheduling, recovery and boundary confirmation, and the new debug metrics.
Avoid force-unwraps and run the closed-loop sequencing tests on the main actor so parallel CI hosts report assertion failures instead of aborting in 0.000s.
Owner
|
Hi @BenjaminD2023, thanks for working on this! Please let me know once you mark this PR as ready for review (open), and I'll jump in to review it. |
Synchronous @mainactor XCTest methods aborted in 0.000s on every parallel CI host. Match the commit-scheduler tests: async methods on the main actor.
Shrink each closed-loop step to about 18% of the selection (36–90 pt) and shorten settle so stitches run more frequently with more overlap. If a commit appends less than half of the expected movement, the next burst uses the smaller retry size. Done always seals the current viewport, because Auto Scroll zeros pending distance and was dropping the last slice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #533
Problem
Scrolling capture with Auto Scroll can produce corrupted stitch seams on long pages with repeated visual structure (GitHub markdown, headings, tables). Two symptoms share the same root cause:
Manual scrolling of the same region usually stitches cleanly.
Root cause
Auto Scroll was an open-loop 40ms wheel timer. Commits were scheduled from opportunistic scroll-event thresholds while the page was still moving. A typical failing session (generation 8, 2026-09-02) was:
expectedDeltaPx=-120,appendDeltaY=60,fast-guided, confidence 0.90.expectedDeltaPx=-111(clamped to last-accepted × 1.85 while Auto Scroll kept posting during a 3834ms stitch),appendDeltaY=468,recovery-vision, confidence 0.373,deltaMagnitudeErrorPx=357. That large false overlap skipped a vertical band.frameAgeMs=1930).lastAcceptedDeltaPixelsthen became the prior for later commits, so one bad match poisoned the rest. Recovery search also ran withexpectedSignedDeltaPixels: nil, so a 468px leap could be accepted against an expected 111px step.Simply slowing the 40ms loop does not close this race: stitch work still takes 1–4s, the ring still holds mid-motion frames, and last-accepted clamping still hides the true step.
Changes
Closed-loop Auto Scroll (
ScrollingCaptureAutoScrollController):idle → emittingBoundedStep → waitingForSettle → requestingCommit → waitingForCommitResult → decidingNextActionexpectedSignedDeltaPixelsis not clamped to the last accepted delta.Narrow stitcher guard: do not let
lastMatchoverride a known current-step expected delta; reject matches that strongly contradict that expected step unless Vision and confidence independently support them.Why this approach
A previous local attempt used closed-loop steps, then a later local commit restored a “cruise” 16ms firehose for smoothness. That cruise path is what produced the 468px skip. Correctness requires a closed loop, not a slower open loop. Step size stays large enough (~34% of the viewport, capped at 260pt) that Auto Scroll still feels continuous.
Automated tests
Targeted
xcodebuild test(Debug, unsigned local run on macOS 26.4.1 / arm64): TEST SUCCEEDED, including:ScrollingCaptureAutoScrollControllerTests(no second step while commit pending, no mid-step commits, stale-frame rejection, expected-delta prior, retry, pointer-outside abort, cancel, Done, two-observation boundary, manual path unsuppressed while idle)ScrollingCaptureStitcherTestsknown-step / intermediate-frame / skipped-band regressions./scripts/run-scrolling-capture-accuracy-benchmark.sh --strict: exit 0A full
xcodebuild testof the Snapzy scheme was not completed in this environment: current Xcode/macOS 26.4.1 treatsSandboxFileAccessManagercalls fromTask.detachedin History views as errors. Those files are unchanged on this branch (CI onmasteris green). Local targeted scrolling-capture tests were run after a temporary local-only hop that was not committed.Manual validation
Environment: macOS 26.4.1 (25E253), Apple M4, mixed-DPI (built-in 2560×1664 Retina + external 5K @ 2× “RV100 Q”), Chrome 152.0.7977.75, Safari 26.4, Snapzy 1.32.0-beta.4 installed + Debug 1.32.0-beta.5 from this branch.
The supplied 1852×2477 capture and
ScrollingCaptureDebugsession-summary from 2026-09-02 18:28 match the skipped-band symptom and the 468px / 0.373 recovery-vision append.Live 5× Auto Scroll of the local repeated-pattern fixture and 3× original-page-type captures still need interactive Screen Recording + Accessibility use of Snapzy Debug after this PR is installed. Deterministic stitcher/controller tests cover the seam classes that live Auto Scroll was producing.
Performance
Closed-loop settle adds ~180ms per step (260ms on retry) plus waiting for post-event frames (timeout 220ms). Step distance is ~34% of the selection height, so total capture time should stay in the same order of magnitude as the old 40ms firehose once stitch time (often 1–4s per commit on this machine) is included. Correctness is prioritized over maximum speed.
Risks and limitations
clean-variable-deltain the accuracy corpus still cannot append every synthetic offset; that predates this change.Screenshots or recordings
No GitHub attachment was added. The original corrupted capture was 1852×2477 with a clipped seam above the ≥200K heading; it is the same session as the debug log quoted above.