Skip to content

test(core): add T10 PreviewAdapter contract stubs (spec for R7)#1262

Merged
vanceingalls merged 4 commits into
mainfrom
06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_
Jun 8, 2026
Merged

test(core): add T10 PreviewAdapter contract stubs (spec for R7)#1262
vanceingalls merged 4 commits into
mainfrom
06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Jun 7, 2026

Summary

  • Adds packages/core/src/studio-api/helpers/previewAdapter.test.ts with 14 it.todo stubs defining the full createPreviewAdapter interface (spec for R7)
  • Follows the T4 pattern (test(studio): add T4 op-contract stubs for editor dispatch boundary #1243): land the interface spec as stubs so the surface is agreed on before implementation begins
  • Covers four describe groups: elementAtPoint (root exclusion, data-hf-id ancestor walk, opacity-at-playhead filter), applyDraft/revertDraft (draft marker lifecycle), commitPreview (patch derivation — move and resize), getElementTimings (data-start/data-end reader)
  • createPreviewAdapter does not exist yet; R7 implements it and converts these stubs to real assertions

Test plan

  • bun run --cwd packages/core test -- previewAdapter.test.ts — 14 todos, 0 failures

@vanceingalls vanceingalls force-pushed the 06-07-refactor_core_extract_maxendtime_serialize_to_parsers_test-utils.ts_tu_ branch from d6ea2e4 to ca90d26 Compare June 8, 2026 01:30
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_ branch from 8130822 to b911db9 Compare June 8, 2026 01:30
miguel-heygen
miguel-heygen previously approved these changes Jun 8, 2026
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

14 it.todo stubs for createPreviewAdapter (R7). The four describe groups map cleanly to distinct surface areas:

  • elementAtPoint — root exclusion, ancestor walk, opacity filter
  • applyDraft/revertDraft — marker lifecycle, --hf-studio-* CSS props
  • commitPreview — null when no gesture, move+resize patches, marker cleanup
  • getElementTimingsdata-start/data-end reads, ignores elements without data-hf-id

The stubs are specific enough to constrain the implementation without over-specifying internals. The opacity-at-playhead stub (elementAtPoint: filters elements with opacity-at-playhead = 0) and the [data-hf-root] exclusion are the two cases most likely to be forgotten in R7 — good that they're explicitly named here.

No issues.

Copy link
Copy Markdown

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-1 review of T10 (PreviewAdapter contract stubs, spec for R7). +51/-0, single file, 14 it.todo stubs across 4 describe groups. Same shape as T4 from the prior batch — stubs land first to lock the surface; R7 converts to real assertions.

What's right

The describe groupings match the natural R7 surface. elementAtPoint, applyDraft/revertDraft, commitPreview, getElementTimings — these are the four orthogonal capabilities a preview-interaction adapter has to expose, and grouping by capability (not by source line) makes the spec readable as a contract rather than a test plan.

Upfront documentation of the geometry-stub gap. Lines 8-12:

Hit-testing (elementAtPoint) in both linkedom and jsdom returns null for all geometry calls — the real tests must inject a position-resolver stub or mock elementFromPoint. The contract tested is filtering logic (root exclusion, data-hf-id ancestor walk, opacity-at-playhead), not geometry.

This is exactly the kind of comment that saves R7's author from spending an hour discovering this on their own. linkedom/jsdom both implement elementFromPoint as a no-op stub, and the tests will need a manual hit-mask. Calling it out before R7 starts is high-leverage.

Stub semantics are concrete. Each it.todo description names a specific behavior rather than a generic capability — "returns the nearest ancestor with data-hf-id", "skips elements whose computed opacity is 0 at the given playhead time". The "given playhead time" parameterization tells R7's author the API needs to accept time as a parameter, which is a real design constraint that's easy to miss otherwise.

Concerns

[concern — applyDraft accepts both move (dx/dy) and resize (w/h) payloads should be two tests] Line 26:

it.todo("applyDraft accepts both move (dx/dy) and resize (w/h) payloads");

When R7 converts this to a real assertion, having both shapes in one test means a failing move assertion masks the resize assertion (and vice versa). The commitPreview group already splits this:

it.todo("derives a moveElement patch from draft markers on commit");
it.todo("derives a resize patch from draft markers on commit");

Either split this applyDraft test the same way, or keep it as one and add a comment that the test body asserts both branches independently. Otherwise R7's author has to make that call mid-implementation.

[concern — missing edge cases worth mentioning before R7 commits to the surface] The current 14 stubs cover the happy path well. Edges that R7 will run into:

  • Concurrent gestures. If applyDraft is called twice in succession before a revertDraft/commitPreview, does the second call overwrite the first's marker, accumulate (dx, dy), or no-op? The current stubs don't pin this down — R7 implementor will pick a behavior and it'll be silently load-bearing for the Studio gesture layer.
  • Gesture interruption mid-flight. Distinct from revertDraft (which is "user cancelled the gesture cleanly") — what if the user starts a drag, then triggers a different action (Esc, scroll-jank, browser tab switch)? Same revertDraft path, or a different cleanup?
  • Playhead motion during a draft. elementAtPoint filters by opacity-at-playhead. If the user holds a drag while scrubbing, does the draft remain valid against the new playhead, or does it re-anchor / get reverted?
  • Stage-root exclusion edge. returns null for the stage root (data-hf-root) — but what if there are nested data-hf-root elements (sub-compositions)? Outer root only, inner-roots count as ancestors, or both excluded?

None are blockers for the stub PR — they're R7-implementation questions. But worth at least one more it.todo per category so the spec captures "yes, R7 has to answer this." A describe("R7 implementation questions — to resolve before converting stubs", () => { it.todo(...) }) block would carry them visibly into the next PR.

Nits

  • getElementTimings has two stubs: read times, ignore elements without data-hf-id. Worth one more for the "what if data-start/data-end is missing on an element with data-hf-id?" path — null, NaN, throw? Tiny gap.
  • The describe-block reads top-to-bottom in roughly the order R7 will need to implement (elementAtPoint first because everything else depends on element lookup, then drafts, then commit, then timings). Good narrative order — keep it.

Plan-doc alignment

T10 = spec for R7 (PreviewAdapter — interaction-layer extraction in Studio → Core). The four describe groups are the four R7 must-haves. ✓ Scope matches.

Verdict

Reasonable spec stub. The applyDraft split and the four edge-case categories are worth adding before R7 starts so the author doesn't have to backtrack the spec mid-implementation. Leaving as a comment.

Review by Rames D Jusso

Extends manualEditsDomPatches.test.ts with rotation and motion pairs.
Same 4-pattern structure: populated, empty, clear restores originals,
build/clear symmetry. Merges duplicate manualEditsTypes import block.
…terization

Fixes four gaps identified in max-setting code review:
- Box-size clear: replace arrayContaining with full ordered toEqual (30 ops)
- Box-size / pathOffset / rotation clear: add empty-string coercion tests
  (origVal||null must produce null, not set property to "")
- Rotation clear: add test for absent STUDIO_ORIGINAL_ROTATION_TRANSFORM_ORIGIN_ATTR
- Motion clear: prove input-independence by calling with both empty and populated
  element and asserting identical output
… (TU)

Deduplicate helpers shared by T1 (htmlParser.roundtrip.test.ts) and T2
(stableIds.test.ts). Both files inline identical implementations; extract
to test-utils.ts so future parser tests (T6a…) import one copy.

Also fix lefthook fallow command to unset GIT_DIR+GIT_INDEX_FILE before
running — those vars are set by git in worktree hook context and block
fallow’s internal temp-worktree creation.
@vanceingalls vanceingalls force-pushed the 06-07-refactor_core_extract_maxendtime_serialize_to_parsers_test-utils.ts_tu_ branch from ca90d26 to 8cf631f Compare June 8, 2026 02:09
All 14 tests are it.todo, following the T4 pattern. The stubs define the
full createPreviewAdapter interface — elementAtPoint (root exclusion,
hf-id ancestor walk, opacity filter), applyDraft/revertDraft (draft
marker lifecycle), commitPreview (patch derivation), and getElementTimings
(data-start/data-end reader).

createPreviewAdapter does not exist yet; R7 implements it and converts
these stubs to real assertions.
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_ branch from b911db9 to 2146fea Compare June 8, 2026 02:09
Base automatically changed from 06-07-refactor_core_extract_maxendtime_serialize_to_parsers_test-utils.ts_tu_ to main June 8, 2026 02:22
@vanceingalls vanceingalls dismissed miguel-heygen’s stale review June 8, 2026 02:22

The base branch was changed.

@vanceingalls vanceingalls merged commit 1fdce71 into main Jun 8, 2026
39 checks passed
@vanceingalls vanceingalls deleted the 06-07-test_core_add_t10_previewadapter_contract_stubs_spec_for_r7_ branch June 8, 2026 02:22
vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
vanceingalls added a commit that referenced this pull request Jun 8, 2026
T6a (#1263): add fromTo corpus script (third parser method) with negative
position values to exercise UnaryExpression arm; drop unused breatheRepeats
from COMPLEX_SCRIPT; generate fromto.parsed.json + fromto.serialized.js goldens.

T10 (#1262): split applyDraft move/resize into two stubs; add applyDraft edge-
case describe block (concurrent gestures, idempotent revert, playhead-change
stability, nested sub-composition root); add getElementTimings stub for absent
data-start/end on a data-hf-id element.

T7 (#1267): expand to full parity with T3 — add text-content, attribute, and
fallthrough stubs (Core sourceMutation supports all patch types via
patchElementInHtml).
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