Skip to content

feat(frontend): add shared row virtualization (WI-2147) - #502

Merged
stefan-ernst merged 2 commits into
mainfrom
feat/wi-2147-shared-virtualization
Nov 13, 2026
Merged

stefan-ernst merged 2 commits into
mainfrom
feat/wi-2147-shared-virtualization

Conversation

@stefan-ernst

Copy link
Copy Markdown
Contributor

Summary

  • Add one domain-independent TanStack virtual-core foundation, keyed rendering, shared viewport subscriptions, anchors, sparse retention and optional drag sessions.
  • Adopt it in tree, list, support queue, flat grouped backlog and paired-pane roadmap; preserve explicit full rendering and print mode.
  • Add an independent native-table catalog example. Board/map remain intentionally unvirtualized.

Status: draft / not release-qualified

WI-2147 remains In Progress. This PR does not waive attachment 916 or claim all acceptance criteria are done.

See frontend/src/lib/virtualization/QUALIFICATION.md for exact commands, evidence and the blocking checklist; README.md beside it documents reuse.

Validation

  • Focused Bun suite: 62 files / 405 tests passed.
  • Browser proofs passed: production-API 1,000-row tree unmount/bounds/full mode; 1,000-row paired-roadmap keys/alignment (<=1px) and either-pane scroll; native backlog offscreen-source auto-scroll/reorder/reload persistence; existing roadmap scheduling, queue, board drag, map swimlanes, backlog story points and roadmap SSE deletion samples.
  • Typecheck: 0 errors/warnings. App/embed/entry-assets and catalog builds passed. Pre-commit frontend/i18n/HMR/token/API/inline-style guards passed.
  • Full Bun suite is NOT green: 484 files passed / 9 failed, 3,644 tests passed / 5 failed. OpenUI/Zod and Vite prebundle failures reproduce on the untouched original checkout with its own lockfile.

Remaining blockers

  • Full cancellation/permission/deletion/partial-failure interaction matrix and larger list/queue/grouped-backlog fixtures.
  • Width/font/RTL/zoom/terminal anchor drift, focus/IME/recovery and navigation restoration qualification.
  • Offscreen-crossing dependency geometry and remaining roadmap pointer invalidation scenarios.
  • Paired baseline/candidate performance, <=20kB unique incremental gzip closure and Safari/WebKit/manual accessibility checks.

Companion private tests PR uses the same branch name and is linked below once created.

@stefan-ernst

Copy link
Copy Markdown
Contributor Author

Companion tests: https://github.com/Windshiftapp/core-tests-internal/pull/46 (overlay commit b3496acc).

Production commit b3ce177 passed the complete pre-commit guard set. Final focused overlay rerun is green: 62 files / 405 tests. Combined browser rerun is still running; its result will be recorded separately. Both PRs intentionally remain draft with WI-2147 In Progress.

@stefan-ernst

Copy link
Copy Markdown
Contributor Author

Final combined qualification on production b3ce177 / tests overlay b3496acc: 21 browser tests passed, including setup; zero retries, zero skips. Focused frontend: 62 files / 405 tests passed.

Exact browser command:

E2E_KEEP_ARTIFACTS=1 ../core-tests/run-e2e.sh tests/virtual-rows.spec.ts tests/backlog-virtual-drag.spec.ts tests/collection-tree-pagination.spec.ts tests/roadmap-scheduling.spec.ts tests/support-queue.spec.ts tests/board-drag-workflow.spec.ts tests/collection-map-swimlanes.spec.ts tests/backlog-story-points-availability.spec.ts tests/collection-roadmap-sse-live.spec.ts

Artifacts retained in the local core-tests e2e/test-results-41886-1794561187 run. The remaining QUALIFICATION.md checklist and baseline-reproduced full-Bun-suite failures still apply. This is not a readiness/completion claim.

@github-actions

Copy link
Copy Markdown

Size Change: 0 B 🆕

Total Size: 0 B

compressed-size-action

@stefan-ernst

Copy link
Copy Markdown
Contributor Author

Reviewed the full diff. Nice piece of work: the controller/adapter split, occurrence keys, sparse pins, shared viewport registry, print mode and the anchor LRU are coherent, and QUALIFICATION.md is unusually honest about what is and isn't proven. A few things I'd like to see addressed or confirmed before this leaves draft.

What I ran locally

  • bun run typecheck → 0 errors / 0 warnings.
  • Biome on the new/changed non-Svelte modules → clean.
  • Focused Vitest: src/lib/virtualization + VirtualRows.test.ts → 27/27 passed; src/lib/features/collections src/lib/features/support src/lib/editors/EditableField.test.js src/lib/pickers → 58 files / 378 passed. That matches the 62-file/405-test claim. I did not run the full suite or the Playwright matrix.

Findings

  1. indexRows turns bad keys into a hard crash (frontend/src/lib/virtualization/rowProjection.ts:11,13). A duplicate or empty key now throws from inside configure, which runs in an effect, so one malformed row takes down the whole collection view. Previously {#each ... (key)} only warned. The derived keys (workspace_id + id, ownerItemId for test cases, backlog projection) make collisions unlikely but not impossible when server data is inconsistent. Consider a non-fatal fallback (e.g. index-qualified key + one-time warning) or at least catch it in VirtualRows so the view degrades instead of blanking.

  2. Shared paired model stores only one DOM node per row key (virtualRowsController.ts:25,331). With the roadmap's sharedModel both panes attach a node for the same key, so nodes ends up holding whichever pane attached last. moveFocus correctly uses paneNodes, but captureAnchor/restoreAnchor (non-virtual mode) and scrollToKey's nodes.has(key) and the drag monitor's controller.nodes.has(key) are pane-blind. For the paired roadmap with ≤100 rows or virtualization off, a non-virtual anchor can be measured from the tree pane while restoreAnchor scrolls the timeline root, producing a wrong restore offset. Either key nodes by (pane, key) or route all non-virtual anchor math through paneNodes.

  3. virtualDragSession commit/cancel state is module-global (virtualDragSession.ts:6-7). committedSources/cancelledSources are keyed by the drag-data object and never cleared. It works today only because every getInitialData returns a fresh object. That is an undocumented contract: any consumer that returns a stable data object will have every drop after the first silently swallowed by claimVirtualDrop. Worth a comment next to the WeakSets, or scoping them to the session/controller.

  4. Scroll anchors are keyed by identity only (scrollRestoration.ts:3). If the same identity is ever rendered by two independent roots (split/secondary pane, design-system preview next to the app), the anchors cross-contaminate; activateVirtualAudience only covers account changes. Probably fine for the current surfaces, but it is a latent constraint worth documenting since identities are caller-supplied strings.

  5. First tree row lost its border suppression (CollectionTree.svelte:642). The old {#each} had {idx === 0 ? 'border-top: none;' : ''}; the virtualized row always sets border-top, so the first visible row now doubles the header's bottom border. Small visual regression — if intended, ignore, otherwise keep the index === 0 case for index 0.

  6. deferred={false} bypasses LazyRender in non-virtual mode too (CollectionList.svelte:319, SupportQueue.svelte:889, backlog rows). When the preference is off or the list is below the 100-row threshold, every row now mounts eagerly; previously LazyRender still deferred offscreen rows. The toggle copy implies full rendering is intentional, but on a large list this is a synchronous full mount. Consider deferred={!virtualization.enabled} so the escape hatch doesn't also remove the previous lazy behavior for small lists.

  7. Identity scopes are inconsistent across views. CollectionList includes query/sort/page, but SupportQueue is queue:${workspaceId}:${activeRef}:${currentPage} (no filter), CollectionTree omits search, and backlog omits collapsed state. That means a filter/search change keeps the same identity and survivingAnchor may restore scroll to a surviving row instead of resetting. If intentional for continuity, fine; otherwise align the scopes.

  8. Roadmap header/body scroll sync (CollectionRoadmap.svelte:1476-1520). Splitting the header into an overflow-hidden container driven by transform: translateX(-timelineScrollLeft) avoids the scroll-margin problem, but it now relies on a JS state mirror of native scroll, which can lag a frame during momentum scrolling, and horizontalScrollbarHeight is only populated by the resize observer so the first paint can be misaligned. Not blocking, but I'd flag it in the qualification notes as a visual-risk area.

  9. Minor: viewportRegistry.subscribeViewport constructs a ResizeObserver unconditionally, while the "no ResizeObserver" guard lives in the controller (virtual getter). It's only reachable when enabled today, but the invariant is split across two modules — a single guard at the registry would be safer. Same class of thing: VirtualRow.svelte creates a $effect inside the attach action; please confirm the effect is disposed when the action is destroyed rather than only with the component.

Qualification

The QUALIFICATION.md blocker list (cancellation/permission/deletion matrix, IME/focus/RTL/zoom anchors, offscreen dependency geometry, paired perf budget, WebKit/accessibility) is the right scope, and the "full suite is not green" disclosure is clear. I wouldn't want this merged as release-qualified until those are closed and the exact proposed commits get the final lint/build/unit/browser pass, but as a draft foundation this looks solid.

@stefan-ernst

Copy link
Copy Markdown
Contributor Author

Addressed the findings in 27f3839, with regression coverage in companion tests PR #46 at b07c91fc. Responding to #502 (comment):

  1. Malformed keys: controller-boundary repair uses collision-safe positional keys for duplicate/empty occurrences and warns once per controller. Valid occurrence keys are unchanged. A rendered malformed-response regression reproduced the crash before the fix. The pure indexRows validator remains strict; positional fallbacks cannot promise reorder-stable editing identity for malformed data.
  2. Paired nodes: removed the pane-blind nodes map. All DOM storage is pane-qualified; nonvirtual anchor and scroll geometry selects the node inside the canonical list. Focus completion can request its originating pane; drag ownership verifies the exact registered node. The adverse attachment-order regression failed with the original -300px offset instead of 8px, and now passes, including independent pane teardown.
  3. Gesture flags: commit/cancel flags now reset on every drag start, so stable drag-data objects work across consecutive gestures. Flags deliberately survive all callbacks of one native drop to preserve one claim per gesture. Reuse-after-commit and reuse-after-Escape regressions failed before the fix and now pass. Teardown is idempotent.
  4. Anchor namespace: documented the contract in README: independent scroll surfaces must prefix identities distinctly, even for identical queries. Root DOM identity is intentionally not the restoration key because it changes on remount. This remains a caller-supplied, audience-scoped namespace.
  5. Tree border: restored suppression for logical index zero using newly exposed snippet index metadata. Regression covers scrolling away/back; later viewport-start rows keep their border.
  6. Eager full rendering: intentionally retained. Reintroducing LazyRender in disabled/normal/print mode would defeat Find/copy/accessibility/print, and deferred={!enabled} would make the explicit escape hatch incomplete. README now states the synchronous O(N) mount cost. The normal-50-row performance acceptance gate is still open, not claimed satisfied.
  7. Identity scopes: queue now includes accepted search, effective queue QL, collection, workspace, page and size, updating with accepted results rather than draft keystrokes. Backlog now uses effectiveBacklogSubFilterQL (the query it actually fetches). Tree has no free-text search input and already scopes its effective filter/hierarchy level. Collapse/expansion intentionally preserve identity and surviving-anchor continuity per attachment 916; this is now documented.
  8. Roadmap sync risk: added explicit first-paint scrollbar compensation and momentum/header-lag risks to QUALIFICATION.md, including required Safari/WebKit checks. Settled <=1px tests are not presented as proving those risks closed.
  9. Registry/lifecycle: the registry now independently tolerates missing ResizeObserver, retaining its resize-event fallback. A regression reproduced the constructor failure before the fix. A separate rendered conditional-node test confirms attachment updates stop after action destruction while the keyed VirtualRow survives and later reorders; the existing Svelte action-owned effect already passes this lifetime contract.

Validation:

  • Same focused Bun overlay command: 63 files / 414 tests passed.
  • Same nine-spec combined Chromium command recorded above: 21 passed, including setup, zero retries/skips; retained run test-results-76593-1794564652.
  • Typecheck: 0 errors / 0 warnings; Biome clean; app/embed/entry-assets build and full pre-commit guards passed.
  • Full frontend suite was not rerun for this follow-up; the earlier baseline-reproduced Bun failures and all remaining release-qualification blockers still apply. Both PRs remain draft; WI-2147 remains In Progress.

@stefan-ernst stefan-ernst changed the title WI-2147: shared row virtualization for collection views feat(frontend): add shared row virtualization (WI-2147) Nov 13, 2026
@stefan-ernst
stefan-ernst marked this pull request as ready for review November 13, 2026 10:24
@stefan-ernst

Copy link
Copy Markdown
Contributor Author

Merging to main at the user’s explicit request after the review follow-up. This integration decision does not mark WI-2147 complete or claim release qualification: the outstanding performance, Safari/WebKit/accessibility and interaction-matrix items in QUALIFICATION.md remain open. Companion tests PR #46 will be merged as well. PR title corrected to satisfy the conventional-title check; no CI bypass requested.

@stefan-ernst
stefan-ernst merged commit 8380f66 into main Nov 13, 2026
22 of 23 checks passed
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