From bf443e8adc7e775eb25cfcf3ad0266a6a0b51fca Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 29 Apr 2026 20:51:04 +0100 Subject: [PATCH 1/2] fix(exposure): cross-variant tracking for create changes (FT-1879) create changes with trigger_on_view previously fell through the generic selector-tracking branch in ExposureTracker, so peer variants without the create had nothing observable and never fired exposure when the would-be position entered viewport. Mirror the move/delete pattern: collect every unique (targetSelector, position) pair across all variants' viewport- triggered create changes and drop an invisible positional placeholder at each. Malformed creates (no targetSelector) are skipped explicitly -- the manipulator can't apply them either. Also gitignore .claude/tasks/ and .worktrees/. --- .gitignore | 2 + ...026-04-29-create-cross-variant-exposure.md | 475 ++++++++++++++++++ src/core/ExposureTracker.ts | 34 +- ...gesPluginLite.crossVariantExposure.test.ts | 254 ++++++++++ 4 files changed, 763 insertions(+), 2 deletions(-) create mode 100644 docs/superpowers/plans/2026-04-29-create-cross-variant-exposure.md diff --git a/.gitignore b/.gitignore index eb9452d..19deb67 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,5 @@ pnpm-lock.yaml node_modules .mcp.json .serena/ +.claude/tasks/ +.worktrees/ diff --git a/docs/superpowers/plans/2026-04-29-create-cross-variant-exposure.md b/docs/superpowers/plans/2026-04-29-create-cross-variant-exposure.md new file mode 100644 index 0000000..c3a34af --- /dev/null +++ b/docs/superpowers/plans/2026-04-29-create-cross-variant-exposure.md @@ -0,0 +1,475 @@ +# Cross-Variant Exposure Tracking for `create` Changes Implementation Plan [FT-1879] + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**JIRA:** FT-1879 — Cross-variant exposure tracking gap for `create` changes with `trigger_on_view` +**PR Title Format:** `fix(exposure): cross-variant tracking for create changes (FT-1879)` + +**Goal:** Ensure SRM-preserving exposure tracking when an experiment uses `create` changes with `trigger_on_view: true` across variants — peer variants (those that don't create the element) must still expose when the user scrolls to the would-be position. + +**Architecture:** Mirror the existing `move`/`delete` pattern: for every unique `(targetSelector, position)` pair across all variants' viewport-triggered `create` changes, drop a 1px invisible positional placeholder via `createContainerPlaceholder`. Dedupe by `target|position` because a created element only exists in the variant that creates it — cross-variant tracking is positional, not selector-based. Malformed `create` changes (missing `targetSelector`) are skipped because `DOMManipulatorLite.applyChange` already returns false for them — no placeholder, no observation, no crash. + +**Tech Stack:** TypeScript, Jest, jsdom, jest-mocked `IntersectionObserver`. Helpers: `createTreatmentTracker` (`src/__tests__/sdk-helper.ts`) and the per-suite `triggerIntersection` defined inline at `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts:104`. + +--- + +## Status of the Fix + +The production fix is **already in place** on this branch: + +- `src/core/ExposureTracker.ts:61-96` — first-pass loop now collects `create` changes into a `createPositions: Map<"target|position", {targetSelector, position}>`. +- `src/core/ExposureTracker.ts:190-204` — after the `delete` block, iterates `createPositions` and calls `createContainerPlaceholder(experimentName, '__create_placeholder__', targetSelector, position)` for each unique pair. +- Malformed `create` (no `targetSelector`) is intentionally skipped inside the new branch — no fallback path. +- `.gitignore` updated to exclude `.claude/tasks/` and `.worktrees/`. + +The remaining work is **test coverage** to lock in the new behavior and protect against regressions. The existing `6F: Create Changes` block at `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts:3972` covers only immediate triggers (`trigger_on_view: false`). Tasks below extend it. + +## File Structure + +| File | Responsibility | +|------|---------------| +| `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` | New `describe` blocks `6F2` – `6F6` covering viewport-triggered create cross-variant scenarios. All additions go inside the existing `6F: Create Changes` block. | + +No production code changes are needed — only tests. Existing fix is locked in; new tests verify it. + +--- + +## Task 1: Baseline — viewport-triggered create in a single variant + +**Files:** +- Modify: `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` — add `describe('6F2: Create with viewport trigger', ...)` after the existing `6F1` block, before the closing `});` of `6F`. + +**Why:** Sanity baseline before the cross-variant cases. Confirms a placeholder is observed and exposure fires when its position enters viewport. + +- [ ] **Step 1: Add the test** + +Insert immediately after the `6F1` closing `});` (around line 4044, before the `6F` closing `});`): + +```typescript + describe('6F2: Create with viewport trigger', () => { + it('user in v1 - exposure fires when target position enters viewport', async () => { + const experiment: ExperimentData = { + name: 'test_6f2_create_viewport', + variants: [ + { variables: {} }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
New
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f2_create_viewport: 1, + }); + document.body.innerHTML = '
Container
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + expect(treatmentSpy).not.toHaveBeenCalled(); + expect(document.querySelector('.new')).not.toBeNull(); + + const placeholder = document.querySelector( + '.container [data-absmartly-placeholder]' + ) as Element; + expect(placeholder).not.toBeNull(); + + await triggerIntersection(placeholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f2_create_viewport'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); +``` + +- [ ] **Step 2: Run only the new test** + +```bash +npx jest --testPathPattern="crossVariantExposure" -t "6F2" +``` + +Expected: 1 passed. + +--- + +## Task 2: Peer variant gets placeholder-based exposure + +**Files:** +- Modify: `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` — add `describe('6F3: Cross-variant create — peer variant', ...)` after `6F2`. + +**Why:** This is the core SRM case the fix addresses. User in control (no create) must still trigger exposure when the position where v1 would have created the element enters viewport. + +- [ ] **Step 1: Add the test** + +Insert after the `6F2` closing `});`: + +```typescript + describe('6F3: Cross-variant create - peer variant placeholder exposure', () => { + it('user in v0 (no create) - exposure fires via placeholder at v1 target position', async () => { + const experiment: ExperimentData = { + name: 'test_6f3_peer_variant', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
New
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f3_peer_variant: 0, + }); + document.body.innerHTML = '
Container
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + // v0 user: nothing was created + expect(document.querySelector('.new')).toBeNull(); + + // ...but a placeholder MUST exist at v1's target position + const placeholder = document.querySelector( + '.container [data-absmartly-placeholder]' + ) as Element; + expect(placeholder).not.toBeNull(); + expect(placeholder.getAttribute('data-absmartly-experiment')).toBe( + 'test_6f3_peer_variant' + ); + + // No exposure until placeholder is visible + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(placeholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f3_peer_variant'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); +``` + +- [ ] **Step 2: Run the new test** + +```bash +npx jest --testPathPattern="crossVariantExposure" -t "6F3" +``` + +Expected: 1 passed. + +--- + +## Task 3: Multiple variants creating at different positions + +**Files:** +- Modify: `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` — add `describe('6F4: Multi-variant create at different positions', ...)` after `6F3`. + +**Why:** Confirms placeholders are dropped at every distinct position and that triggering ANY placeholder fires exposure. Three-variant case to flush out off-by-one issues in the dedup map. + +- [ ] **Step 1: Add the test** + +```typescript + describe('6F4: Multi-variant create at different positions', () => { + it('user in v0 - placeholders at every other variant position; any one fires exposure', async () => { + const experiment: ExperimentData = { + name: 'test_6f4_multi_position', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.header', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.footer', + position: 'firstChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f4_multi_position: 0, + }); + document.body.innerHTML = + '
H
F
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const headerPlaceholder = document.querySelector( + '.header [data-absmartly-placeholder]' + ) as Element; + const footerPlaceholder = document.querySelector( + '.footer [data-absmartly-placeholder]' + ) as Element; + + expect(headerPlaceholder).not.toBeNull(); + expect(footerPlaceholder).not.toBeNull(); + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(footerPlaceholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f4_multi_position'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); +``` + +- [ ] **Step 2: Run the new test** + +```bash +npx jest --testPathPattern="crossVariantExposure" -t "6F4" +``` + +Expected: 1 passed. + +--- + +## Task 4: Same-position dedup across variants + +**Files:** +- Modify: `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` — add `describe('6F5: Same-position create dedup', ...)` after `6F4`. + +**Why:** Two variants creating different elements at the same `(targetSelector, position)` must produce exactly one placeholder. The `createPositions` Map is keyed by `target|position`, so this verifies the dedup contract. + +- [ ] **Step 1: Add the test** + +```typescript + describe('6F5: Same-position create dedup', () => { + it('user in v0 - exactly one placeholder when two variants create at same target/position', async () => { + const experiment: ExperimentData = { + name: 'test_6f5_dedup', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f5_dedup: 0, + }); + document.body.innerHTML = '
C
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const placeholders = document.querySelectorAll( + '.container [data-absmartly-placeholder]' + ); + expect(placeholders.length).toBe(1); + + await triggerIntersection(placeholders[0], true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f5_dedup'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); +``` + +- [ ] **Step 2: Run the new test** + +```bash +npx jest --testPathPattern="crossVariantExposure" -t "6F5" +``` + +Expected: 1 passed. + +--- + +## Task 5: Malformed create (no `targetSelector`) is a no-op + +**Files:** +- Modify: `src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts` — add `describe('6F6: Malformed create — no targetSelector', ...)` after `6F5`. + +**Why:** Confirms the explicit skip in the new branch. A `create` without `targetSelector` cannot be applied by `DOMManipulatorLite` (returns false at `DOMManipulatorLite.ts:190`). The tracker must not crash, must not create a placeholder, and (when no other tracking exists) must not fire exposure. + +- [ ] **Step 1: Add the test** + +```typescript + describe('6F6: Malformed create - no targetSelector', () => { + it('user in v0 - no placeholder, no exposure, no crash', async () => { + const experiment: ExperimentData = { + name: 'test_6f6_malformed', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + // targetSelector intentionally omitted + trigger_on_view: true, + } as any, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f6_malformed: 0, + }); + document.body.innerHTML = '
C
'; + + // Setup must not throw + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + // No placeholder anywhere for this experiment + const placeholder = document.querySelector( + '[data-absmartly-experiment="test_6f6_malformed"]' + ); + expect(placeholder).toBeNull(); + + // The element was never created either + expect(document.querySelector('.orphan')).toBeNull(); + + // No exposure - nothing observable means nothing to trigger + expect(treatmentSpy).not.toHaveBeenCalled(); + }); + }); +``` + +- [ ] **Step 2: Run the new test** + +```bash +npx jest --testPathPattern="crossVariantExposure" -t "6F6" +``` + +Expected: 1 passed. + +--- + +## Task 6: Full suite verification + commit + +**Files:** +- All staged. + +- [ ] **Step 1: Run full plugin test suite** + +```bash +npx jest --testPathPattern="DOMChangesPluginLite" +``` + +Expected: 7 passed test suites, ≥ 259 passed (254 baseline + 5 new), 1 skipped. + +- [ ] **Step 2: Run lint** + +```bash +npm run lint 2>&1 | tail -20 +``` + +Expected: no errors. (Pre-commit prettier hook will reformat on commit anyway.) + +- [ ] **Step 3: Stage changes** + +```bash +git add .gitignore src/core/ExposureTracker.ts src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts docs/superpowers/plans/2026-04-29-create-cross-variant-exposure.md +git status +``` + +Expected status: only the four files above modified/added. + +- [ ] **Step 4: Commit** + +```bash +git commit -m "$(cat <<'EOF' +fix(exposure): cross-variant tracking for create changes (FT-1879) + +create changes with trigger_on_view previously fell through the generic +selector-tracking branch in ExposureTracker, so peer variants without the +create had nothing observable and never fired exposure when the would-be +position entered viewport. Mirror the move/delete pattern: collect every +unique (targetSelector, position) pair across all variants' viewport- +triggered create changes and drop an invisible positional placeholder at +each. Malformed creates (no targetSelector) are skipped explicitly -- +the manipulator can't apply them either. + +Also gitignore .claude/tasks/ and .worktrees/. +EOF +)" +``` + +- [ ] **Step 5: Push** + +```bash +git push -u origin fix/FT-1879/create-cross-variant-exposure +``` + +Report the branch URL back to the user. PR creation is left to the user (or a follow-up `/create-pr` invocation). + +--- + +## Self-Review Notes + +- **Spec coverage:** Acceptance criteria from FT-1879 — (1) placeholder per unique position ✓ (Tasks 2–5 verify); (2) tests across the four scenarios ✓ (Tasks 2/3/4/5); (3) existing suites pass ✓ (Task 6 step 1). +- **Placeholders:** none — every test step contains complete code and exact run commands. +- **Type consistency:** `data-absmartly-placeholder` attribute, `data-absmartly-experiment` attribute, and the `__create_placeholder__` originalSelector sentinel are all set in `ExposureTracker.createContainerPlaceholder` (`src/core/ExposureTracker.ts:283-286`). Tests query by attribute, not by value, so the sentinel can change without breaking tests. diff --git a/src/core/ExposureTracker.ts b/src/core/ExposureTracker.ts index 87c9a3c..4aa0581 100644 --- a/src/core/ExposureTracker.ts +++ b/src/core/ExposureTracker.ts @@ -58,10 +58,13 @@ export class ExposureTracker { const viewportSelectors = new Set(); const moveParentSelectors = new Set(); // Parent containers for move changes - // First pass: collect all move and delete changes across all variants + // First pass: collect all move, delete, and create changes across all variants // We need to track ALL possible positions where elements could be const moveElements = new Map>(); // selector -> Set of target parent positions const deleteElements = new Set(); // selectors for delete changes + // For create: dedupe by "targetSelector|position" since a created element only + // exists in the variant that creates it — cross-variant tracking is positional. + const createPositions = new Map(); for (const variantChanges of allVariantsChanges) { for (const change of variantChanges) { @@ -78,8 +81,19 @@ export class ExposureTracker { } else if (change.type === 'delete') { // Track delete changes - need special handling for placeholders deleteElements.add(change.selector); + } else if (change.type === 'create') { + // create requires targetSelector to have a DOM position; without it + // the manipulator can't apply the change at all (DOMManipulatorLite + // returns false), so there's nothing to track. + if (change.targetSelector) { + const position = change.position || 'lastChild'; + const key = `${change.targetSelector}|${position}`; + if (!createPositions.has(key)) { + createPositions.set(key, { targetSelector: change.targetSelector, position }); + } + } } else { - // For other non-move, non-delete changes, track the selector directly + // For other change types, track the selector directly viewportSelectors.add(change.selector); } } @@ -178,6 +192,22 @@ export class ExposureTracker { } } + // For cross-variant create tracking: drop an invisible placeholder at every + // (targetSelector, position) where any variant creates an element with + // trigger_on_view. This guarantees exposure fires when the user scrolls to + // the position regardless of whether their variant actually created the + // element. The user's variant that does create the element gets the real + // element in DOM at the same position (applyChange runs before this), and + // the 1px invisible placeholder coexists harmlessly. + for (const [, pos] of createPositions) { + this.createContainerPlaceholder( + experimentName, + '__create_placeholder__', + pos.targetSelector, + pos.position + ); + } + // Trigger flags are now passed in from DOMChangesPluginLite after URL filtering // This ensures only variants matching the current URL determine trigger behavior diff --git a/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts b/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts index 34187be..3de5ab2 100644 --- a/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts +++ b/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts @@ -4041,6 +4041,260 @@ describe('DOMChangesPluginLite - Comprehensive Cross-Variant Exposure Tracking', expect(document.querySelector('.new')).not.toBeNull(); // Created in v1 }); }); + + describe('6F2: Create with viewport trigger', () => { + it('user in v1 - exposure fires when target position enters viewport', async () => { + const experiment: ExperimentData = { + name: 'test_6f2_create_viewport', + variants: [ + { variables: {} }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
New
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f2_create_viewport: 1, + }); + document.body.innerHTML = '
Container
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + expect(treatmentSpy).not.toHaveBeenCalled(); + expect(document.querySelector('.new')).not.toBeNull(); + + const placeholder = document.querySelector( + '.container [data-absmartly-placeholder]' + ) as Element; + expect(placeholder).not.toBeNull(); + + await triggerIntersection(placeholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f2_create_viewport'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F3: Cross-variant create - peer variant placeholder exposure', () => { + it('user in v0 (no create) - exposure fires via placeholder at v1 target position', async () => { + const experiment: ExperimentData = { + name: 'test_6f3_peer_variant', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
New
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f3_peer_variant: 0, + }); + document.body.innerHTML = '
Container
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + expect(document.querySelector('.new')).toBeNull(); + + const placeholder = document.querySelector( + '.container [data-absmartly-placeholder]' + ) as Element; + expect(placeholder).not.toBeNull(); + expect(placeholder.getAttribute('data-absmartly-experiment')).toBe( + 'test_6f3_peer_variant' + ); + + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(placeholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f3_peer_variant'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F4: Multi-variant create at different positions', () => { + it('user in v0 - placeholders at every other variant position; any one fires exposure', async () => { + const experiment: ExperimentData = { + name: 'test_6f4_multi_position', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.header', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.footer', + position: 'firstChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f4_multi_position: 0, + }); + document.body.innerHTML = '
H
F
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const headerPlaceholder = document.querySelector( + '.header [data-absmartly-placeholder]' + ) as Element; + const footerPlaceholder = document.querySelector( + '.footer [data-absmartly-placeholder]' + ) as Element; + + expect(headerPlaceholder).not.toBeNull(); + expect(footerPlaceholder).not.toBeNull(); + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(footerPlaceholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f4_multi_position'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F5: Same-position create dedup', () => { + it('user in v0 - exactly one placeholder when two variants create at same target/position', async () => { + const experiment: ExperimentData = { + name: 'test_6f5_dedup', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f5_dedup: 0, + }); + document.body.innerHTML = '
C
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const placeholders = document.querySelectorAll('.container [data-absmartly-placeholder]'); + expect(placeholders.length).toBe(1); + + await triggerIntersection(placeholders[0], true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f5_dedup'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F6: Malformed create - no targetSelector', () => { + it('user in v0 - no placeholder, no exposure, no crash', async () => { + const experiment: ExperimentData = { + name: 'test_6f6_malformed', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + trigger_on_view: true, + } as any, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f6_malformed: 0, + }); + document.body.innerHTML = '
C
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const placeholder = document.querySelector( + '[data-absmartly-experiment="test_6f6_malformed"]' + ); + expect(placeholder).toBeNull(); + + expect(document.querySelector('.orphan')).toBeNull(); + + expect(treatmentSpy).not.toHaveBeenCalled(); + }); + }); }); describe('6G: Edge Cases', () => { From 63f1851e2ff0f05c7612a815b820ac26afc4fe6c Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Wed, 29 Apr 2026 22:29:59 +0100 Subject: [PATCH 2/2] fix(exposure): handle delayed-mount targets and harden create tracking (FT-1879) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on PR #5: - Delayed-mount fallback (CodeRabbit, code-reviewer, test-analyzer, silent-failure-hunter all flagged this): when targetSelector is not in the DOM at registration time (SPA late-mount), createContainerPlaceholder now returns false and the create-positions loop falls back to adding the targetSelector to viewportSelectors, so the existing MutationObserver picks up exposure when the target eventually appears. - Collision-safe key: replace the "${target}|${position}" composite map key with a nested Map> so CSS selectors that legally contain '|' (namespace, attribute selectors) cannot collide. - Debug log when a malformed create (no targetSelector) is skipped, so the silent skip can be discovered when debug=true. Test additions: - 6F3 extended with cleanup verification (placeholder removed, no re-fire). - 6F4 second test exercises the OTHER (header) placeholder. - 6F6 second test confirms a malformed create does not poison registration of well-formed sibling changes in the same variant. - 6F7 (new): delayed-mount targetSelector — exposure fires after target is appended later via MutationObserver. - 6F8 (new): URL-filter combined with viewport-create cross-variant tracking, both v0 and v1 users. Suite: 263 passed (was 259), 1 skipped, 0 failures, 0 lint errors. --- src/core/ExposureTracker.ts | 60 +++-- ...gesPluginLite.crossVariantExposure.test.ts | 219 ++++++++++++++++++ 2 files changed, 263 insertions(+), 16 deletions(-) diff --git a/src/core/ExposureTracker.ts b/src/core/ExposureTracker.ts index 4aa0581..f79f95a 100644 --- a/src/core/ExposureTracker.ts +++ b/src/core/ExposureTracker.ts @@ -62,9 +62,11 @@ export class ExposureTracker { // We need to track ALL possible positions where elements could be const moveElements = new Map>(); // selector -> Set of target parent positions const deleteElements = new Set(); // selectors for delete changes - // For create: dedupe by "targetSelector|position" since a created element only - // exists in the variant that creates it — cross-variant tracking is positional. - const createPositions = new Map(); + // For create: cross-variant exposure is positional (the created element only + // exists in its own variant). Nested map avoids any composite-key collision — + // CSS selectors can legally contain characters like '|' or ':', so we key + // structurally instead of stringifying the pair. + const createPositions = new Map>(); // targetSelector -> Set for (const variantChanges of allVariantsChanges) { for (const change of variantChanges) { @@ -87,10 +89,17 @@ export class ExposureTracker { // returns false), so there's nothing to track. if (change.targetSelector) { const position = change.position || 'lastChild'; - const key = `${change.targetSelector}|${position}`; - if (!createPositions.has(key)) { - createPositions.set(key, { targetSelector: change.targetSelector, position }); + let positions = createPositions.get(change.targetSelector); + if (!positions) { + positions = new Set(); + createPositions.set(change.targetSelector, positions); } + positions.add(position); + } else if (this.debug) { + logDebug( + `[EXPOSURE] [${experimentName}] Skipping create change with no targetSelector — manipulator can't apply it either`, + { change } + ); } } else { // For other change types, track the selector directly @@ -199,13 +208,28 @@ export class ExposureTracker { // element. The user's variant that does create the element gets the real // element in DOM at the same position (applyChange runs before this), and // the 1px invisible placeholder coexists harmlessly. - for (const [, pos] of createPositions) { - this.createContainerPlaceholder( - experimentName, - '__create_placeholder__', - pos.targetSelector, - pos.position - ); + // + // Fallback: if the targetSelector isn't in the DOM at registration time + // (SPA late-mount), createContainerPlaceholder returns false. Track the + // targetSelector itself so the existing MutationObserver picks up exposure + // when the target eventually appears. + for (const [targetSelector, positions] of createPositions) { + let anyInserted = false; + for (const position of positions) { + if ( + this.createContainerPlaceholder( + experimentName, + '__create_placeholder__', + targetSelector, + position + ) + ) { + anyInserted = true; + } + } + if (!anyInserted) { + viewportSelectors.add(targetSelector); + } } // Trigger flags are now passed in from DOMChangesPluginLite after URL filtering @@ -278,21 +302,23 @@ export class ExposureTracker { /** * Create a container-based placeholder at the hypothetical position * Uses inline-block with minimal dimensions to be observable but not affect layout + * Returns true when a placeholder was inserted (or already exists for this key); + * false when the target element isn't in the DOM yet, so callers can fall back. */ private createContainerPlaceholder( experimentName: string, originalSelector: string, targetSelector: string, position: string = 'lastChild' - ): void { + ): boolean { const targetElement = document.querySelector(targetSelector); - if (!targetElement) return; + if (!targetElement) return false; const placeholderKey = `${experimentName}-${originalSelector}-${targetSelector}-${position}`; // Check if placeholder already exists if (this.placeholders.has(placeholderKey)) { - return; + return true; } // Create minimal placeholder using inline styles @@ -343,6 +369,8 @@ export class ExposureTracker { `[ABsmartly] Created placeholder for ${originalSelector} at ${targetSelector} (${position})` ); } + + return true; } /** diff --git a/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts b/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts index 3de5ab2..0c7e0f3 100644 --- a/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts +++ b/src/core/__tests__/DOMChangesPluginLite.crossVariantExposure.test.ts @@ -4135,6 +4135,12 @@ describe('DOMChangesPluginLite - Comprehensive Cross-Variant Exposure Tracking', expect(treatmentSpy).toHaveBeenCalledWith('test_6f3_peer_variant'); expect(treatmentSpy).toHaveBeenCalledTimes(1); + + // Cleanup contract: after exposure fires, placeholder is removed from + // the DOM, and a second intersection does not re-trigger treatment. + expect(document.querySelector('.container [data-absmartly-placeholder]')).toBeNull(); + await triggerIntersection(placeholder, true); + expect(treatmentSpy).toHaveBeenCalledTimes(1); }); }); @@ -4199,6 +4205,61 @@ describe('DOMChangesPluginLite - Comprehensive Cross-Variant Exposure Tracking', expect(treatmentSpy).toHaveBeenCalledWith('test_6f4_multi_position'); expect(treatmentSpy).toHaveBeenCalledTimes(1); }); + + it('user in v0 - intersecting the OTHER placeholder also fires exposure', async () => { + const experiment: ExperimentData = { + name: 'test_6f4_multi_position_b', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.header', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '', + targetSelector: '.footer', + position: 'firstChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f4_multi_position_b: 0, + }); + document.body.innerHTML = '
H
F
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + const headerPlaceholder = document.querySelector( + '.header [data-absmartly-placeholder]' + ) as Element; + expect(headerPlaceholder).not.toBeNull(); + + await triggerIntersection(headerPlaceholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f4_multi_position_b'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); }); describe('6F5: Same-position create dedup', () => { @@ -4294,6 +4355,164 @@ describe('DOMChangesPluginLite - Comprehensive Cross-Variant Exposure Tracking', expect(treatmentSpy).not.toHaveBeenCalled(); }); + + it('does not poison registration of well-formed sibling changes in the same variant', async () => { + const experiment: ExperimentData = { + name: 'test_6f6_malformed_mixed', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + trigger_on_view: true, + } as any, + { + selector: '.text-target', + type: 'text', + value: 'Changed', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f6_malformed_mixed: 0, + }); + document.body.innerHTML = + '
C
T
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + // Malformed create produced no placeholder, but the well-formed sibling + // change is still tracked. + expect(treatmentSpy).not.toHaveBeenCalled(); + const textTarget = document.querySelector('.text-target')!; + await triggerIntersection(textTarget, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f6_malformed_mixed'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F7: Cross-variant create - delayed-mount targetSelector', () => { + it('user in v0 - exposure fires once targetSelector is appended later (SPA late-mount)', async () => { + const experiment: ExperimentData = { + name: 'test_6f7_delayed_target', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.late-container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + ], + }; + + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f7_delayed_target: 0, + }); + // .late-container is NOT in the DOM at registration time. + document.body.innerHTML = '
Other
'; + + plugin = new DOMChangesPluginLite({ context: mockContext, autoApply: true, spa: false }); + await plugin.ready(); + + // Placeholder couldn't be inserted (target absent); fallback registers + // the targetSelector itself with the MutationObserver. + expect(document.querySelector('[data-absmartly-placeholder]')).toBeNull(); + expect(treatmentSpy).not.toHaveBeenCalled(); + + // Target appears later (e.g. SPA navigation / hydration). + const lateContainer = document.createElement('div'); + lateContainer.className = 'late-container'; + document.body.appendChild(lateContainer); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(lateContainer, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f7_delayed_target'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('6F8: Cross-variant create with URL filter', () => { + it('user in v0 - exposure fires when URL matches v1 filter (variant has create + filter)', async () => { + const currentUrl = 'https://example.com/products'; + Object.defineProperty(window, 'location', { + value: { href: currentUrl }, + writable: true, + }); + + const experiment: ExperimentData = { + name: 'test_6f8_url_filter', + variants: [ + { variables: { __dom_changes: [] } }, + { + variables: { + __dom_changes: { + urlFilter: { include: ['/products'] }, + changes: [ + { + selector: '', + type: 'create', + element: '
', + targetSelector: '.container', + position: 'lastChild', + trigger_on_view: true, + }, + ], + }, + }, + }, + ], + }; + + for (const userVariant of [0, 1]) { + const { mockContext, treatmentSpy } = createTreatmentTracker([experiment], { + test_6f8_url_filter: userVariant, + }); + document.body.innerHTML = '
C
'; + + plugin = new DOMChangesPluginLite({ + context: mockContext, + autoApply: true, + spa: false, + }); + await plugin.ready(); + + const placeholder = document.querySelector( + '.container [data-absmartly-placeholder]' + ) as Element; + expect(placeholder).not.toBeNull(); + expect(treatmentSpy).not.toHaveBeenCalled(); + + await triggerIntersection(placeholder, true); + + expect(treatmentSpy).toHaveBeenCalledWith('test_6f8_url_filter'); + expect(treatmentSpy).toHaveBeenCalledTimes(1); + + plugin.destroy(); + } + }); }); });