perf(studio): resolve a selector's occurrence index once per layer walk

[miguel-heygen]

What

The preview flattens several composition files into one DOM, so a layer's identity is its selector plus its occurrence index within its own source file. getSourceScopedSelectorIndex derived that index per element: a whole-document querySelectorAll(selector), resolveSourceFile on every match, then indexOf.

Every element sharing a class paid for all of them. A walk over n such elements did n whole-document queries and n² source-file resolutions — which is the shape a composition of repeated cards or tiles has by construction.

This builds the occurrence index once per selector and shares it across one walk. withSelectorIndexPass(doc, run) opens that scope; outside it the helper behaves exactly as before, per call.

Where the fix lives

In collectDomEditLayerItems, which owns the loop — not at a call site. Its four callers all walk the same way and all paid the same cost:

caller	what it drives
offCanvasIndicatorGeometry	off-canvas indicator rebuild (throttled, runs while anything animates)
LayersPanel	the layers list
marqueeCommit	marquee hit-testing
webmcp/tools/lookTools	the agent's look tool

Fixing only the first would have left three siblings quadratic.

Behaviour

Unchanged. The occurrence indices are identical, including the misses: an element outside the requested source file, or one not matching the selector, still yields undefined, as do #-prefixed and [data-composition-id= selectors and an invalid selector.

sourceScopedSelectorIndex.test.ts checks this against the previous algorithm transcribed verbatim as a reference, over every (element, scope) pair in a document where several elements share a class across two different source files — the case source-file scoping exists for.

Tests assert complexity invariance, not a threshold

The query count must be identical at n and at 4n elements sharing a selector. A threshold tuned to a small fixture passes while a real composition runs hundreds of elements; invariance fails for any per-element term at all.

Both invariance tests fail on the previous algorithm (48 queries where 12 are expected) and pass with the shared index. Guards sit at two levels: on collectDomEditLayerItems, which owns the fix and therefore covers all four callers, and on recomputeOffCanvasIndicators, which proves it through the real production entry point.

Measured

Driving a real Studio over 80 single-frame seeks on a 1689-element preview (25 nested compositions, several classes shared by 25–140 elements), counting querySelectorAll against the preview document:

per rebuild	before	after
class-selector document queries	171.6	10.8
walk cost, self-timed	15.25 ms	5.37 ms
.word queries (140 elements share it)	140	1
elements walked	973.74	973.79
[data-composition-id] lookups (untouched)	969.2	969.9

Both arms ran against the same server and page, toggling only this memo, so the last two rows are the controls: equal work walked, and a metric this change does not touch staying put.

What this does not prove. One machine, one composition shape, under other load — the millisecond figure is noisy and the call counts are the trustworthy half. It is the walk's self-timed cost, not end-to-end seek latency, and it is not a measured frame-rate improvement. The size of the win scales with how many elements share a class, so a composition with unique ids per element sees none of it.

Left out deliberately

orientedGroupAwareOverlayRect resolves the composition root with a querySelector("[data-composition-id]") per element (~970 per rebuild above, the largest single remaining term). It is O(n), not O(n²), and hoisting it means threading a precomputed scale through three exported geometry functions — a wider diff and a separate correctness argument about rect staleness across a walk. Worth doing next, on its own.

Also noted and not attempted here: the rebuild is dirtied by a MutationObserver on style/class/transform, i.e. exactly what an animation runtime writes every frame, and responds by rebuilding the whole preview. Recomputing only the changed subtree is a real improvement and a different seam with its own correctness argument.

Verification

• packages/studio: 435 files, 4807 tests passing.
• Root lint and format:check clean; tsc --noEmit clean.

[terencecho]

APPROVE — hf#3831 hoists the source-scoped selector-index resolution from once-per-element to once-per-selector-per-walk. Correctness preserved (verified against a verbatim reference impl in tests), all four walk-callers covered via the owning function collectDomEditLayerItems, all CI green, no concurrent reviews.

Author + trust. Sole commit fbba7ed5 authored by Miguel Ángel (miguel-heygen, miguel.sierra@heygen.com) — trust-listed, no bot co-authorship. Head fbba7ed50362d0e184b2943f73fac1e4615c005a. mergeStateStatus=BLOCKED, mergeable=MERGEABLE.

1. Cache scope — the per-walk correctness axis.

• activePass is a module-level singleton, but withSelectorIndexPass saves the previous pass on entry and restores it in finally (sourceScopedSelectorIndex.ts:35-43). Nested passes over different docs are correct — verified by sourceScopedSelectorIndex.test.ts:127-146 ("does not reuse one document's index for another, and restores the outer pass").
• getSourceScopedSelectorIndex gates the cache lookup on activePass?.doc === doc (sourceScopedSelectorIndex.ts:74) — a nested pass over a different document skips the cache entirely rather than poison it. Cross-document leakage impossible.
• Behaviour outside a pass: falls through to buildOccurrences per call — identical query count to the pre-fix per-call algorithm (one querySelectorAll per call), just now via the new Map construction path instead of Array.from(...).filter(...).indexOf(...). No perf regression for un-passed callers.

2. Miss-semantics parity (the subtle contract).
The old algorithm returned undefined in four distinct miss cases; each preserved:

• Guarded selectors (undefined, #-prefixed, [data-composition-id=) → early undefined at line 68 (unchanged).
• Invalid selector → try/catch wraps buildOccurrences, returns undefined (line 84, same as before).
• Element not in doc's matches at all → occurrences.get(el) returns undefined → returned (line 82).
• Element IS a match but in a different scope than requested → hit && hit.scope === (sourceFile ?? "index.html") guard returns undefined (line 82) — this matches the old filter(...).indexOf(el) === -1 behaviour where the wrong-scope el was filtered out before indexOf.

Verified end-to-end by sourceScopedSelectorIndex.test.ts:84-96 — every (element × scope) pair including undefined, "index.html", "a.html", "b.html", "missing.html" compared to the verbatim reference impl at lines 8-28.

3. Four callers, one owner.
Grepped collectDomEditLayerItems across the repo — all four consumer sites go through it and inherit the pass:

• packages/studio/src/components/editor/offCanvasIndicatorGeometry.ts:60 (throttled indicator rebuild).
• packages/studio/src/components/editor/LayersPanel.tsx:140 (the panel).
• packages/studio/src/components/editor/marqueeCommit.ts:46 (drag hit-test).
• packages/studio/src/webmcp/tools/lookTools.ts:181 (agent look tool).

Placing the pass in collectDomEditLayerItems rather than each caller is the right level — every caller pays the same cost pre-fix and gets the same win.

4. Walk is read-only (docstring precondition).
visit in domEditingLayers.ts:459-486 calls getDomLayerPatchTarget, buildElementLabel, getDirectLayerChildren and pushes to a local items[]. Zero DOM mutation. groupScopedLayerRoots is a read-only filter. Precondition "run must not mutate doc" holds.

5. Resolver consistency inside the pass.
The cache is keyed by selector alone, so a second call inside the pass with a different resolver would silently reuse the first's classification. Checked: the walk's only call site is getDomLayerPatchTarget → getSelectorIndex → the resolver (candidate) => getSourceFileForElement(candidate, activeCompositionPath).sourceFile (domEditingDom.ts:300-303). activeCompositionPath is a scalar captured for the walk — deterministic within the pass.

The OTHER getSourceScopedSelectorIndex call site — timelineElementHelpers.ts:278 with resolver getTimelineElementSourceFile — is called only from timelineDOM.ts / timelineIframeHelpers.ts, never from the layers walk. No cache pollution possible today. Worth guarding if a future walk starts crossing subsystems, but that's not this PR's problem.

6. Test coverage — pins the invariant.
Unusually thorough:

• sourceScopedSelectorIndex.test.ts (148 lines) — reference-impl comparison test. The old per-element algorithm is transcribed verbatim at lines 8-28; every (element, scope) pair is compared against the memoized result both inside and outside a pass.
• domEditingLayers.test.ts:264-306 — classSelectorQueries(48) === classSelectorQueries(12) === 1. Invariance across a 4× fixture. Fails deterministically on the pre-fix code (per the PR body's "48 queries where 12 are expected" claim).
• offCanvasIndicatorGeometry.complexity.test.ts (117 lines) — end-to-end through the real production entry recomputeOffCanvasIndicators, asserting [[".box", 1], [".label", 1]] at both n=12 and n=48, and that keys stay index.html:.box:${i} in document order. Proves both the pass reaches the walk AND the shared index still numbers correctly.

Both invariance tests pass at n and 4n with the same query count — the exact property a fixture-sized threshold would miss.

7. Perf-claim plausibility.
Claim: 171.6 → 10.8 class queries per rebuild (~16×). Test asserts classSelectorQueries(12) === 1. On a real 1689-element preview with .word shared by 140 elements, the pre-fix per-element cost is dominated by the highest-cardinality classes — 140 → 1 for .word alone accounts for ~135 of the 161-query delta. Ratio is consistent with the code change.

8. CI. All required checks GREEN at head:

• CI: Build, Lint, Typecheck, Test, Format, Fallow audit, File size check, Semantic PR title, all Producer/SDK/Runtime/Studio sub-checks — SUCCESS.
• Preview parity, preview-regression, player-perf, regression, Windows render verification, CodeQL, WIP — SUCCESS.
• SKIPPED lanes (Preflight in player-perf/regression/preview-regression, GCP BeginFrame image contract, Skills/CLI shim, Codex plugin) are all Detect changes-gated: those workflows short-circuit lanes irrelevant to the diff. Correct skip pattern, not green-by-skip anti-pattern — Preflight for touched code did run (CI's Lint + Typecheck + Format).
• No red, no in-progress.

9. Scope / blast-radius.

• +387 / -8 across 5 files, all under packages/studio. Dominated by tests (~313 lines).
• No new external deps, no public API changes, no other tests removed/modified.
• No interaction with #3824's composition-duration cache work — different code paths.

10. Reviews. reviews array empty. No concurrent bot or human review at head.

Minor observations, non-blocking.

• The module-level activePass implicitly dynamic-scopes the cache. If a caller later adds await inside withSelectorIndexPass, post-await work silently loses the cache (drops back to per-call). Author's docstring is explicit ("ONE synchronous walk"), and collectDomEditLayerItems is fully synchronous today. Fine for now, worth remembering if the walk ever needs async work.
• Author flags two adjacent wins deliberately left out (orientedGroupAwareOverlayRect composition-root lookup, and the whole-preview MutationObserver rebuild). Good scoping call — separate correctness arguments, would balloon the diff.

— tai

[vanceingalls]

APPROVE. Verified independently against source at fbba7ed5 (not just the diff/description), plus tai's review which landed first — my read converges with tai's on every point, and I additionally built the workspace and ran the actual test suite rather than reading it.

1. Behavior preservation — traced, not assumed.
buildOccurrences (sourceScopedSelectorIndex.ts:57-70) does one querySelectorAll(selector) in document order and assigns each match a per-scope running index. That's exactly equivalent to the old Array.from(doc.querySelectorAll(selector)).filter(scope-match).indexOf(el), because grouping-while-preserving-document-order reproduces the same per-scope ordinal a post-hoc filter+indexOf would. The four miss cases (guarded selector, invalid selector, element absent from any match, element matched but in the wrong scope) all map onto the new code identically — confirmed by reading both branches side by side, not just trusting the reference-impl test.

2. All four callers verified, not just grepped.
Traced collectDomEditLayerItems's only consumers — LayersPanel.tsx:143, marqueeCommit.ts:48, offCanvasIndicatorGeometry.ts:60, webmcp/tools/lookTools.ts:181 — all funnel through the same walk, so the pass and its correctness apply uniformly; nothing bypasses it. offCanvasIndicatorGeometry.ts itself is untouched by this diff (only its test file is new), and it still calls orientedGroupAwareOverlayRect (the [data-composition-id] ancestor-lookup Miguel scoped out for a future PR) — confirmed that code path is not touched or altered here.

3. Resolver-consistency caveat — non-blocking, same as tai flagged.
The module-level activePass cache keys only on selector string. getSourceScopedSelectorIndex has two real call sites with genuinely different resolveSourceFile closures — domEditingDom.ts:300 (via getSourceFileForElement, checks the element itself + activeCompositionPath fallback) vs. timelineElementHelpers.ts:287 (via getTimelineElementSourceFile, checks only el.parentElement, no fallback). If a future caller ever opened a pass on the same doc while the timeline path ran underneath it, results would silently corrupt (wrong cached scope, no error). Today it's unreachable: withSelectorIndexPass is opened only inside collectDomEditLayerItems, and the timeline path never nests inside that walk. Worth a defensive comment near activePass for future maintainers, but not a blocker for this PR.

4. Ran the tests, didn't just read them.
Built @hyperframes/{parsers,lint,studio-server,core} in a worktree and ran the suite directly:

• sourceScopedSelectorIndex.test.ts, domEditingLayers.test.ts, offCanvasIndicatorGeometry.complexity.test.ts — 28/28 pass, including the two complexity-invariance tests (classSelectorQueries(48) === classSelectorQueries(12) === 1).
• LayersPanel.test.ts (15) and lookTools.test.ts (16) — both consumers of the changed walk — pass.
• tsc --noEmit on packages/studio — clean.
• Note: a full unfiltered bun run test in this worktree throws ~1487 unrelated act is not a function failures across the player/timeline test files — a local React/test-utils resolution issue in this sandbox, not caused by this diff (none of the failing files touch the changed code, and GitHub CI's own full run is green). Flagging in case anyone else hits it locally.
• marqueeCommit.ts has no dedicated test file at all (pre-existing gap, not introduced or worsened here).

5. Perf claim is plausible from the code shape. One querySelectorAll per distinct selector via Map-based occurrence building matches the claimed ~171.6 → ~10.8 queries/rebuild order of magnitude; the .word-shared-by-140 case collapsing to 1 query accounts for most of the delta.

No blockers. Solid, well-scoped, well-tested perf fix — behavior-preserving, and deliberately does not touch the separate ancestor-lookup or MutationObserver work reserved for future PRs.

— Vai