Skip to content

test(core,studio): add T3+T7 hfId targeting stubs (spec for R1)#1267

Merged
vanceingalls merged 6 commits into
mainfrom
06-07-test_core_studio_add_t3_t7_hfid_targeting_stubs_spec_for_r1_
Jun 8, 2026
Merged

test(core,studio): add T3+T7 hfId targeting stubs (spec for R1)#1267
vanceingalls merged 6 commits into
mainfrom
06-07-test_core_studio_add_t3_t7_hfid_targeting_stubs_spec_for_r1_

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Jun 8, 2026

Summary

  • T3 (packages/studio/src/utils/sourcePatcher.test.ts): 5 it.todo stubs for PatchTarget.hfId targeting — inline style patch, text content patch, attribute patch, hfId preservation after patch, and fallthrough when hfId not found
  • T7 (packages/core/src/studio-api/helpers/sourceMutation.test.ts): 2 it.todo stubs for SourceMutationTarget.hfId — basic style patch by data-hf-id and data-hf-id attribute survival after patch

Neither interface has hfId yet. R1 adds the field to both PatchTarget and SourceMutationTarget, implements the [data-hf-id="…"] lookup branch in findTagByTarget / findTargetElement, then converts these stubs to real assertions.

Test plan

  • bun run --cwd packages/studio test -- sourcePatcher.test.ts — 24 pass, 5 todo
  • bun run --cwd packages/core test -- sourceMutation.test.ts — 37 pass, 2 todo

@vanceingalls vanceingalls force-pushed the 06-07-test_core_studio_add_t3_t7_hfid_targeting_stubs_spec_for_r1_ branch from 2b9c6ba to e926c74 Compare June 8, 2026 01:30
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ branch from e2a90ee to 534b77d 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.

5 stubs in sourcePatcher.test.ts (T3) and 2 in sourceMutation.test.ts (T7) for the hfId targeting path.

The stubs cover the right cases for R1:

  • PatchTarget.hfId: inline style, text content, attribute ops — the three op types applyPatch handles
  • hfId preservation after patch — important since the patch applies to source HTML and could in principle clobber data-hf-id if the rewriter isn't careful
  • Fallthrough to selector when hfId not found — this is the key contract boundary: fail gracefully, don't silently no-op

The corresponding SourceMutationTarget.hfId stubs in T7 are leaner (2 stubs vs 5), which makes sense — sourceMutation goes through findTargetElement which is a single lookup vs sourcePatcher's more complex AST rewriting path.

P3 nit: the stub "uses [data-hf-id=...] selector when resolving target" in T7 slightly pre-specifies the implementation (bracket-attribute selector). If R1 uses a different internal lookup (e.g. querySelector without necessarily constructing a CSS selector string), the stub name could cause confusion. Consider: "resolves element by hfId attribute".

No blockers. The two existing test suites (sourcePatcher.test.ts should have 24 pass + 5 todo; sourceMutation.test.ts 37 pass + 2 todo) must pass with 0 failures for CI to be green.

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 T3 + T7 (hfId targeting stubs, spec for R1). +24/-0, two files: 5 it.todo stubs for PatchTarget.hfId in Studio's sourcePatcher, 2 it.todo stubs for SourceMutationTarget.hfId in Core's sourceMutation.

What's right

Stubs are paired to the R1 implementation points. PR body names them: findTagByTarget in Studio and findTargetElement / sourceMutation.ts:34 in Core. Both functions live where you'd expect the lookup branch to land — keeping the spec stubs in the same test files as the existing selector/id tests means R1's PR-author flips them from it.todo to it(...) without scaffolding new infrastructure.

T3's fallthrough case is the critical one. Line 533:

it.todo("hfId lookup falls through to selector when hfId not found");

This is the load-bearing semantic for migration safety. If R1 lands and existing compositions don't yet have data-hf-id attrs, the new code path has to gracefully fall back to selector-based targeting. Pinning this as an explicit test in the spec means R1 can't ship without the fallback. ✓

T3's "hfId attribute is preserved after a patch" guards the round-trip invariant. Same shape as the existing motion attribute round-trip via sourcePatcher test block above it — if a patch strips the data-hf-id attr accidentally, subsequent patches lose the addressability and the editor's reference graph breaks.

Concerns

[concern — T3 / T7 surface asymmetry needs a one-line explanation] T3 has 5 stubs covering style, text, and attribute patch types plus preservation and fallthrough. T7 has 2 stubs covering only style patch + attribute survival. This is either:

(a) SourceMutationTarget only supports a narrower patch surface than PatchTarget — in which case 2 stubs is correct AND the PR body should say "T7 is intentionally narrower because Core's sourceMutation only exposes the style branch over the data-hf-id path", or

(b) SourceMutationTarget supports the full surface and T7 is missing three stubs (text-by-hfId, attribute-by-hfId, and the fallthrough case).

The PR body says T7 covers "basic style patch by data-hf-id and data-hf-id attribute survival after patch" — "basic" implies (a), but doesn't confirm it. Worth a one-line clarification so R1's PR author doesn't have to dig through sourceMutation.ts to figure out which branches need test coverage. If (b), this PR is missing three stubs.

Also: T7 has no fallthrough stub. If the Core surface DOES need fallback semantics ("hfId-not-found falls through to the selector branch"), R1 will land without that test pinned. If it doesn't (because Core mutation always has an id available by contract), that's worth noting in the test file.

[nit — stub naming inconsistency between T3 and T7] T3 uses imperative-active voice ("updates inline style by data-hf-id"). T7 uses the same pattern but for one case writes "patches element by data-hf-id" rather than "updates element by data-hf-id" — and the second T7 stub uses noun-phrase voice ("data-hf-id attribute survives the patch"). Tiny noise; converging on one voice across both files (e.g. "updates X via data-hf-id" and "data-hf-id is preserved after a Y") makes the stubs feel like one spec rather than two.

Plan-doc alignment

T3 + T7 are explicit specs for R1 (the hfId?: string field + [data-hf-id="..."] lookup branch on both PatchTarget and SourceMutationTarget). Stub-only structure matches the T4 / T10 pattern in this stack. Test plan checklist (24 pass, 5 todo / 37 pass, 2 todo) confirms the stubs compile and the existing suites still pass.

Verdict

Clean stub PR. The T3/T7 asymmetry is the only thing worth resolving before this lands — if (a) above is correct, one sentence in the file or PR body documents it; if (b), three more stubs in sourceMutation.test.ts close the gap. Either way, the answer is short. 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.
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.
6 toMatchFileSnapshot tests across 3 representative scripts (minimal,
moderate, complex). Captures parseGsapScript + serializeGsapAnimations
output before the Recast → Meriyah swap so any parser change is detected
as a golden diff rather than a silent behavioral regression.

Goldens live in src/parsers/__goldens__/ and are checked in. Add
__goldens__/** to fallow ignorePatterns (data files, not modules) and to
.prettierignore so oxfmt does not reformat vitest-written snapshot files.
@vanceingalls vanceingalls force-pushed the 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ branch from 534b77d to 4095bfc Compare June 8, 2026 02:21
T3 (sourcePatcher.test.ts): 5 it.todo stubs for PatchTarget.hfId targeting
— style, text, attribute patches plus preservation and fallthrough cases.

T7 (sourceMutation.test.ts): 2 it.todo stubs for SourceMutationTarget.hfId
— basic patch and data-hf-id survival after patch.

Neither interface has hfId yet. R1 adds the field + [data-hf-id="…"] branch
in findTagByTarget / findTargetElement, then converts these to real assertions.
@vanceingalls vanceingalls force-pushed the 06-07-test_core_studio_add_t3_t7_hfid_targeting_stubs_spec_for_r1_ branch from e926c74 to 564bd4b Compare 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).
Base automatically changed from 06-07-test_core_add_t6a_gsap_parser_golden_baselines_recast_babel_snapshot_ to main June 8, 2026 02:23
@vanceingalls vanceingalls dismissed miguel-heygen’s stale review June 8, 2026 02:23

The base branch was changed.

@vanceingalls vanceingalls merged commit 1e705b1 into main Jun 8, 2026
20 checks passed
@vanceingalls vanceingalls deleted the 06-07-test_core_studio_add_t3_t7_hfid_targeting_stubs_spec_for_r1_ branch June 8, 2026 02:23
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