Skip to content

test(scroll-sync): cover the split-view mapping by running it - #442

Merged
PathGao merged 1 commit into
masterfrom
test/extract-scroll-sync-math
Aug 3, 2026
Merged

test(scroll-sync): cover the split-view mapping by running it#442
PathGao merged 1 commit into
masterfrom
test/extract-scroll-sync-math

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Scroll sync is the miss from the coverage audit that is easiest to close, because the part of it that can be wrong is arithmetic.

The injected defect set the sync threshold to a value that disabled sync entirely. previewScrollSync.test.ts (23 assertions) and scrollSyncInput.test.ts stayed green through it. Both read a .svelte file as text, so what they detect is that a particular line was edited — the first was deleted in #433 for exactly that. The behaviour is currently untested.

That is not a property of those two files. node --test cannot import .svelte, so 14,581 lines across 21 components have no executable coverage at all, and the only thing a test can do to logic inside one is grep it. The fix that works is the one that produced documentSession and windowSession: move the logic out. This is one function pair's worth of that.

What moved, and the check that it moved unchanged

getScrollSyncPositionFromPixels and getScrollTopForSyncPosition read nothing but their arguments — no DOM, no Monaco, no component state, no closure over anything but the private clampScrollRatio beside them. They are pure arithmetic and leave the component with no wrapper and no seam.

They were also duplicated. Both functions and clampScrollRatio existed byte for byte in Editor.svelte (editor side) and MarkdownViewer.svelte (preview side) — two private copies of one formula, each invisible to the other and to the compiler. Nothing imports a private function, so the two could have drifted silently; a fix applied to one pane's copy would have left the other pane mapping by the old formula, and split view would have desynchronised in one direction only. That is the bug class singleImplementationConvention.test.ts exists for, and it could not see this one because a private identifier is not a legal marker there.

Both call sites now import src/lib/utils/scrollSync.ts. The diff on both components is a deletion plus an import; Editor.svelte's editorReady effect from #423 is untouched.

The move is byte-identical, not asserted to be. Taking the two pre-change blocks out of git show HEAD:, removing the module's export keywords and re-indenting it by one tab:

Editor.svelte            md5=31ab31100fc37e77e4a08d0c18470e70  identical_to_module=True
MarkdownViewer.svelte    md5=31ab31100fc37e77e4a08d0c18470e70  identical_to_module=True
utils/scrollSync.ts      md5=31ab31100fc37e77e4a08d0c18470e70

What the mapping is for

The two panes are not proportional to each other. Front matter is ordinary text lines in the editor and a fixed-height panel in the preview, so the same document gives them different total scroll ranges and a different share of that range spent on front matter. The tests use 1000px total / 200px front matter for the editor and 600px / 60px for the preview — 20% against 10%.

A single scrollTop / scrollMax ratio drifts under that. At the moment the editor finishes scrolling past its front matter, (200 / 1000) * 600 = 120px in the preview is already 60px into the body. Splitting the range at frontMatterEnd and carrying a (section, ratio) pair instead maps front matter onto front matter, body onto body, and the boundary onto the boundary — 60, exactly.

That number is the first assertion in the file, with the naive 120 computed alongside it so the test says why 60 is the answer.

Tests

scripts/scrollSync.test.ts, 12 tests, all of them calling the functions. No readFileSync, no regex over source.

test what can go wrong
the front-matter boundary in one pane lands on the boundary in the other the drift above; the mapping stops being section-aware
front matter maps onto front matter by its own share of the section front-matter positions land by global ratio
body maps onto body by its share of the body range, not of the whole pane frontMatterEnd dropped from the body arithmetic
top and bottom are fixed points of the mapping the ends stop agreeing
scrolling one pane down always moves the other pane down a constant return — the shape of the defect that was missed
a position round-trips through pixels in the pane it came from the mapping is not invertible where it must be
the boundary pixel belongs to the body, not to the front matter < / <= at the section split
a document with no front matter is one body range frontMatterEnd of 0
a document shorter than the viewport has no scroll range and no division by zero scrollMax of 0
front matter taller than the scroll range leaves no body range frontMatterEnd >= scrollMax, zero body range
out-of-range pixels clamp instead of escaping the pane negative and past-the-end inputs
a non-finite measurement or ratio never reaches setScrollTop NaN out of a mid-layout measurement

Two of those are worth stating in full, because they are not restatements of the implementation.

The round trip is exact, including where the mapping looks degenerate. For a scrollTop on either side of the boundary, position → pixels returns the same pixel and pixels → position returns the same position. It holds even when the body range is zero (frontMatterEnd >= scrollMax), because a body ratio of 0 resolves to frontMatterEnd, which in that pane is the bottom of the scroll range.

The boundary label can only be asserted as a label. At scrollTop === frontMatterEnd, frontmatter at ratio 1 and body at ratio 0 resolve to the same pixel in every pane, so flipping < to <= is invisible in the target scrollTop. Asserting section === 'body' there is not pedantry about the return shape: it is what stops a one-pixel scroll from jumping back to a ratio of 1.

Mutation check

Each defect injected into src/lib/utils/scrollSync.ts on top of this change, the suite run, the file restored. 12 tests in the suite.

injected defect result first red test
sync disabled: position is always the top RED (10) front matter maps onto front matter by its own share of the section
sync disabled: target scrollTop is always 0 RED (11) the front-matter boundary in one pane lands on the boundary in the other
identity: ratio taken over the whole pane, frontMatterEnd dropped RED (5) the front-matter boundary in one pane lands on the boundary in the other
frontMatterEnd dropped from the reverse mapping RED (7) the front-matter boundary in one pane lands on the boundary in the other
inverted ratio (forward) RED (6) front matter maps onto front matter by its own share of the section
inverted ratio (reverse, body) RED (8) the front-matter boundary in one pane lands on the boundary in the other
section never reported as front matter RED (7) front matter maps onto front matter by its own share of the section
off-by-one at the boundary: < becomes <= RED (2) the boundary pixel belongs to the body, not to the front matter
boundary clamp dropped: frontMatterEnd not capped at scrollMax RED (3) a document shorter than the viewport has no scroll range and no division by zero
scrollTop not clamped to the scroll range RED (1) a non-finite measurement or ratio never reaches setScrollTop
zero-range guard dropped: division by a zero body range RED (4) a document shorter than the viewport has no scroll range and no division by zero
non-finite ratio guard dropped RED (1) a non-finite measurement or ratio never reaches setScrollTop
ratio not clamped in the reverse mapping RED (1) a non-finite measurement or ratio never reaches setScrollTop

13 injected, 13 caught. The first two rows are the two ways to write "sync does nothing", which is the defect the deleted tests could not fail.

Messages name the measurement, not the line:

editor boundary must land on the preview boundary
editor 199 -> 200 must move the preview forward, got 59.7 -> 59.7
ratio NaN in frontmatter produced NaN

The first pass had a fourteenth entry that came back green. It was a bad patch, not a gap — the injected early return sat behind if (false && …) and never executed. Rewritten to actually fire, it is row 1.

npm test 540 → 552 passing, npm run check 633 files / 0 errors, npm run build clean.

Not covered

  • revealFoldsAround / foldToggleFor in FindBar.svelte — the same extraction, and the closest of the three. foldToggleFor(container, root) is already parameterised on the DOM it touches; revealFoldsAround closes only over markdownBody and a module constant, so passing the root in makes it pure over a DOM interface. The behaviour worth pinning is an ordering claim — outermost fold opened first, so a nested one is on screen when its own toggle fires — and foldMeasurementBatching.test.ts's recording stand-in already records exactly that kind of ordered event stream. Needs a stand-in for classList/closest/matches/dispatchEvent and a CSS.escape shim, so a larger fixture than this PR's, but the same shape.
  • exportAsPdf in export.ts — importable already, so no extraction; the blocker is different in kind. It calls invoke and the dialog plugin's save as module-level imports, so a test needs the window.__TAURI_INTERNALS__ stub that checkedReadMigration.test.ts and reopenDirtyDocument.test.ts already use, plus a seam for save. The claims are small and real: non-Windows invokes print_pdf and never opens a dialog, Windows opens one, and a cancelled dialog must invoke nothing. macosPdfExport.test.ts covers the command-name half by grep and nothing covers the branch.
  • reloadFromDisk's dirty-state guard in MarkdownViewer.svelte — not the same clean extraction: it has no arithmetic core, and what matters is the sequencing (canCloseTab resolves the buffer, then loadMarkdown runs with discardUnsavedBuffer: true to cover typing during the await). A predicate lifted out of it would not carry that. The realistic route is the documentSession one — canCloseTab and loadMarkdown are both already in documentSession.svelte.ts, which several test files drive for real against a stubbed backend, so moving the reload beside them makes it testable without a new technique. That is a behaviour-moving change with its own risk, not a mechanical lift, and belongs in its own PR.
  • No guard against the duplication coming back. singleImplementationConvention.test.ts could take a row pinning getScrollSyncPositionFromPixels to scrollSync.ts, and now that the name is exported it would be a legal marker. Not added here: it is a source-text assertion, this PR's claim is that executable tests are what catch defects, and the two should not be argued in the same change.
  • No live split-view run. The mapping is verified against the real functions; the measurement functions that feed it (getEditorContentScrollMax, getEditorFrontMatterScrollEnd, getPreviewFrontMatterScrollEnd) still live in components, still read Monaco and the DOM, and are still uncovered. A wrong frontMatterEnd would produce a correct mapping of a wrong number, and nothing here would notice.

🤖 Generated with Claude Code

Scroll sync had 23 assertions and no coverage. The defect that set the sync
threshold to a value which disabled sync outright left every one of them green,
because they read Editor.svelte as text; both files were deleted in #433, so the
behaviour is currently untested.

The cause is structural: `node --test` cannot import a `.svelte` file, so the
only thing a test can do to logic living inside one is grep it. The two
functions that decide where the other pane scrolls to are pure arithmetic —
`getScrollSyncPositionFromPixels` and `getScrollTopForSyncPosition` read nothing
but their arguments — so they can simply leave the component, the same move that
produced `documentSession` and `windowSession`.

They were also duplicated. Both functions plus their `clampScrollRatio` helper
existed byte for byte in Editor.svelte and MarkdownViewer.svelte: two private
copies of one formula, invisible to each other and to the compiler. Both call
sites now import `src/lib/utils/scrollSync.ts`, whose body is the moved code
verbatim apart from `export` (md5 of all three blocks, de-indented: 31ab3110).

scripts/scrollSync.test.ts imports and runs it. The cases are stated as a
mapping between two panes with different proportions — the editor spends 20% of
its scroll range on front matter, the preview 10% — because that asymmetry is
the reason the position carries a section instead of a single ratio. 13 injected
defects, 13 caught, including both spellings of "sync does nothing".

540 -> 552 tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant