fix(studio): shadow resolves bare leaf via dispatch path, not getElement

[vanceingalls]

Summary

The resolver-parity shadow tripwire (packages/studio/src/utils/sdkResolverShadow.ts) was emitting false element_not_found divergences for any element edited inside an inlined sub-composition. PostHog showed ~445 such events — all one user editing an inlined yt-lower-third (selecting #subscribe-btn / hf-0ytc). The real cutover path would have resolved these fine; the shadow used the wrong resolver.

This PR makes the shadow resolve ids the same way the cutover dispatch path does, eliminating the false positives. No behavior change to the SDK or to the user-visible edit path — telemetry-only correctness fix.

Root cause

Two resolvers exist in the SDK, with deliberately different semantics:

• resolveScoped (engine/model.ts) — used by the dispatch path (mutate.ts, every op handler). For a bare id it does a whole-document querySelectorAll, so it locates a leaf anywhere (canonical instance preferred, else first match). This is what cutover persist actually uses.
• Composition.getElement (session.ts) — canonical-only for a bare id by design: it will not resolve a bare id to a non-canonical (sub-composition) element, so that removeElement(bareId) and getElement(bareId) agree on the same instance (session.subcomp.test.ts "ambiguous bare id" suite, e.g. the post-removal null at line 472).

When a composition inlines a sub-composition (host element carries data-composition-file, opening a new scope), a child element gets scopedId = "hf-host/hf-leaf" — but its raw data-hf-id attribute (what the studio reads off the live DOM and passes around) stays the bare leaf (hf-0ytc).

The studio selection flows the bare leaf id everywhere:

• Cutover dispatch → resolveScoped(bareLeaf) → finds the scoped element → works.
• Shadow → session.getElement(bareLeaf) → canonical-only → null → emits element_not_found.

So the shadow reported a divergence the real dispatch path never experiences. The pristine standalone yt-lower-third block has no host boundary (scopedId === id), so it resolves fine — which is why this only reproduced against an inlined composition, not the registry block.

Reproduction

host (data-composition-file="sub.html")  →  new scope
  └─ leaf (data-hf-id="hf-leaf")          →  scopedId "hf-host/hf-leaf"

session.getElement("hf-leaf")        → null    ← shadow saw this → false element_not_found
session.getElement("hf-host/hf-leaf")→ FOUND
resolveScoped(doc, "hf-leaf")        → FOUND    ← what dispatch actually uses

Fix

Add resolveSnapshot(session, id) in the shadow module, mirroring resolveScoped against the element snapshots: exact scopedId match → canonical bare match → first bare match. Use it at all three element_not_found sites (sdkResolverShadowCheck pre-guard + post-dispatch read-back, and recordResolverParity).

getElement is unchanged — its canonical-only contract is correct and load-bearing for removeElement/getElement agreement. (Verified: adding a leaf fallback to getElement directly breaks the "ambiguous bare id" suite by resurrecting the inner dup.)

Why fix the shadow, not getElement

The shadow's job is to detect whether the SDK would resolve what the studio selected the way dispatch resolves it. Dispatch uses resolveScoped. So the faithful check is resolveScoped-equivalent. getElement answers a different question ("which instance does a bare id canonically own") and must keep doing so.

Tests

New group H. inlined sub-composition leaf in sdkResolverShadow.test.ts:

• documents the trap: getElement(bareLeaf) is null while getElement(scoped) resolves;
• recordResolverParity emits nothing for a bare leaf inside a sub-comp;
• sdkResolverShadowCheck does not flag element_not_found for that case.

C8's mock updated to expose getElements (the resolver now reads snapshots). All 59 tests pass (shadow + session.subcomp). Lint, format, fallow (0 issues on changed files), typecheck, and full build all green.

Impact

• PostHog studio:sdk_resolver_shadow / element_not_found noise stops; the tripwire now flags only genuine divergences.
• No change to SDK public API, cutover behavior, or the on-disk edit path. STUDIO_SDK_RESOLVER_SHADOW_ENABLED stays default-on (telemetry-only).

🤖 Generated with Claude Code

[vanceingalls]

• fix(studio): shadow resolves bare leaf via dispatch path, not getElement #1552 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Review: fix(studio): shadow resolves bare leaf via dispatch path, not getElement

Excellent write-up, Vance. The root-cause analysis in the PR body is thorough — the dual-resolver semantics (resolveScoped vs getElement) and the inlined sub-composition scope boundary that exposes the mismatch are both clearly documented.

The fix

resolveSnapshot mirrors resolveScoped semantics against the snapshot array: exact scopedId match first, then canonical bare-id match, then first bare-id match. This is exactly the three-tier priority the dispatch path uses, and it replaces the three call sites where the shadow was incorrectly using getElement (which deliberately refuses to resolve bare ids to non-canonical scoped elements).

What I like

1. Surgical scope. Two files, telemetry-only path, zero user-visible behavior change. The blast radius is as small as it can be.
2. getElement is untouched. The PR body explains why — its canonical-only contract is load-bearing for removeElement/getElement agreement, and the ambiguous bare id test suite guards that. Correct call not to touch it.
3. Test section H is well-structured. Three tests that document the trap (getElement returns null for a bare leaf in a sub-comp), prove recordResolverParity no longer fires, and prove sdkResolverShadowCheck no longer flags element_not_found. The INLINED_HTML fixture clearly sets up the host→leaf scope boundary.
4. C8 mock fix. Adding getElements: () => [] to the C8 mock is necessary since resolveSnapshot now calls getElements() — without it the mock would blow up. Good catch.

Observations (non-blocking)

1. resolveSnapshot does a linear scan twice — find for exact, then filter + find/index for bare. For the shadow telemetry path this is fine (element counts are small, call frequency is low), but if this ever moves to a hot path, a single pass would be cleaner:

   let canonical: FlatEl | null = null;
   let first: FlatEl | null = null;
   for (const el of els) {
     if (el.scopedId === id) return el;
     if (el.id === id) {
       if (el.scopedId === el.id) canonical = el;
       else first ??= el;
     }
   }
   return canonical ?? first ?? null;
   

   Not worth changing here — just noting for if this function ever gets reused.

2. No negative test for a genuinely missing element. Section H covers the false-positive elimination. It might be worth a quick test confirming that a truly non-existent id ("hf-nope") still correctly produces element_not_found through the new resolveSnapshot path, just to pin the other direction. Optional — the existing C8 test covers this with the mock, but an integration-level test with openComposition would be belt-and-suspenders.

3. The resolveSnapshot JSDoc is excellent — it explains the why behind the three-tier fallback and explicitly references resolveScoped and the session.subcomp.test suite. This is the kind of comment that saves someone 30 minutes six months from now.

Verdict

Clean, well-scoped fix with solid test coverage. The telemetry noise (~445 false element_not_found events) gets eliminated without touching the SDK public surface or the getElement contract. The observations above are non-blocking polish items.

LGTM — ready for stamp.

— Miga

[miguel-heygen]

Verified the diff. resolveSnapshot correctly mirrors resolveScoped's 3-tier resolution (exact scopedId → canonical bare → first bare match), replacing getElement at all 3 shadow callsites. Section H tests reproduce the exact false-positive (bare leaf inside inlined sub-comp where getElement returns null but dispatch resolves fine). Existing test C8 updated to include getElements in the stub. LGTM.

[jrusso1020]

Approved at 873d5835 per Rames D Jusso R1 + Miga LGTM. Surgical telemetry-only correctness fix on top of #1547.

Diagnosis is excellent. Two resolvers, deliberately different semantics: resolveScoped whole-document, Composition.getElement canonical-only by design (for removeElement/getElement agreement on the post-removal null contract). The shadow was using the wrong one for the question it actually answers — "what would dispatch see?" — and emitting ~445 false element_not_found events against a single user editing inlined yt-lower-third. The fix mirrors the dispatch resolver shape (resolveSnapshot: exact scopedId → canonical bare → first bare) against snapshots; getElement's canonical-only contract is preserved untouched.

Verified:

• All three swap sites updated (sdkResolverShadowCheck pre-guard, post-dispatch readback, recordResolverParity). Zero remaining session.getElement(hfId) in sdkResolverShadow.ts.
• Composition.getElement + session.subcomp.test.ts are zero-diff vs main — "ambiguous bare id" suite intact.
• Section H tests cover both the false-positive-eliminated direction (bare leaf in sub-comp emits nothing) AND the true-positive-preserved direction (genuinely-missing id still emits via the C8 empty-snapshots case).
• STUDIO_SDK_RESOLVER_SHADOW_ENABLED default-on unchanged; PostHog event shape unchanged — emits less often, schema identical.

Non-blocking observations (follow-up territory):

• Echoing Rames D Jusso: resolveScoped's 2nd fallback (no data-hf-id match → consult data-composition-id at model.ts:87) isn't mirrored in resolveSnapshot. Currently theoretical — Studio's cutover paths pass bare data-hf-id from selection, not comp-id — but if future callers address sub-comp roots by comp-id, the resolver gap would silently re-emerge.
• Test smell: A1's vi.spyOn(session, "getElement") is now a stale witness (shadow no longer touches getElement); assertion still passes but doesn't prove what it used to. Not worth chasing.

Stack-tip clean. Ready to merge once the remaining required check (Tests-on-windows re-run) lands.