fix(studio): per-child patch op builders and persist-seam harness

[miguel-heygen]

Part 4/7 of the design-panel fix stack (replaces #1888).

What

Client half of the per-child patch-op contract introduced in #1908, plus the headless integration harness that locks the whole seam down.

• buildTextFieldChildLocator builds :scope > tag locators with the child's index computed over the parent's full same-tag child list (a previousElementSibling walk on the preview element), matching exactly how the server resolves — a same-tag non-leaf sibling can no longer shift the index onto the wrong element.
• buildTextFieldChildOperations emits per-field inline-style/text-content ops for same-shape multi-field edits, and returns null for structural changes (add/remove field, mixed text nodes) so callers can refuse instead of corrupting.
• shouldUseSdkCutover declines any batch containing a child-scoped op — the SDK path maps ops by hfId only and would silently target the parent.
• persistSeam.integration.test.ts: real client patch builders driven through the real patchElementInHtml against the test(studio): add design-panel QA fixture and triage matrix #1906 fixture — per-section style ops, timing/media attributes, sub-composition template targets, runtime-node matched:false, identical-sibling disambiguation, and the non-leaf-sibling index regression.
• Vite resolve.alias entry for the ./source-mutation subpath (same pattern as the existing player alias).

Testing

domEditTextFieldCommitOps.test.ts, sdkCutoverEligibility.test.ts, and the 12-case persist-seam harness; full studio suite green at this stack level (1281 tests).

[miguel-heygen]

• test(studio): agent-browser e2e smoke for the design panel #1912
• test(studio): cover persist-failure hook behavior #1911
• fix(studio): surface persist failures with toast and guarded revert #1910
• fix(studio): per-child patch op builders and persist-seam harness #1909 👈 (View in Graphite)
• fix(studio-server): child-scoped patch operations with batch abort #1908
• fix(studio): make canvas selection hit intended elements #1907
• test(studio): add design-panel QA fixture and triage matrix #1906
• main

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

[james-russo-rames-d-jusso]

Reviewed at da95739c02d51d7b5c97d50b1ae16992d6572b19.
Peer scan: no prior reviews on this PR at review time.
Stack context: this PR against studio-panel-3-server-child-ops (part 4/7); paired with #1908 (walked both together for wire coherence). Second-pass lens per James's ask; Via has runtime-interop.

Summary — Client half of the child-scoped patch-op contract. buildTextFieldChildLocator builds :scope > <tag> locators indexed over the PARENT's full same-tag child list (via sameTagChildIndex(el), a previousElementSibling walk on the live preview element), matching #1908's server-side parent.querySelectorAll(':scope > <tag>')[childIndex] resolution. buildTextFieldChildOperations emits per-field inline-style/text-content ops for same-shape multi-field edits and returns null for structural changes (add/remove field, mixed text-nodes, reorder) so callers refuse instead of corrupting. persistSeam.integration.test.ts drives real client builders through the real patchElementInHtml via the new @hyperframes/studio-server/source-mutation subpath export from #1908 — this is the load-bearing bit that makes the "producer parity" real, not reflexive.

Concerns

• 🔴 BLOCKER — sdkCutoverEligibility.ts is an orphan module; the child-scoped gate is in the wrong file. The PR body promises "shouldUseSdkCutover declines any batch containing a child-scoped op — the SDK path maps ops by hfId only and would silently target the parent." The gate is added to packages/studio/src/utils/sdkCutoverEligibility.ts:117-118. But that file is not imported by any production code. The runtime hot path calls shouldUseSdkCutover in packages/studio/src/utils/sdkCutover.ts:104-119, a DUPLICATE that this PR did not touch. Trace: useDomEditCommits.ts:171 → onTrySdkPersist → sdkCutoverPersist (sdkCutover.ts:225) → shouldUseSdkCutover(...) at sdkCutover.ts:234 → returns TRUE for child-scoped ops → patchOpsToSdkEditOps(hfId, ops) (sdkOpMapping.ts:15-42) drops childSelector entirely (every op is emitted with target: hfId, the parent). Result: with the SDK cutover flag on + an active sdkSession, an editable-multi-line text edit routes through the SDK path, which applies the child style to the PARENT element — reintroducing the exact "corrupt one line, hit both" symptom this stack is fixing. Verified grep -rn "sdkCutoverEligibility" packages/ returns zero importers; sdkCutoverEligibility.test.ts imports from ./sdkCutoverEligibility too, so the new test at :12-16 verifies the orphan copy and passes without protecting the live path. Fix options: (a) delete sdkCutoverEligibility.ts and add the hasChildScopedOp gate to sdkCutover.ts:104-118 (single source of truth), (b) make sdkCutover.ts re-export the orphan's shouldUseSdkCutover and drop its local copy, (c) at minimum for this PR: inline the gate at sdkCutover.ts:110-118 and add the same test against sdkCutover.ts so the guardrail can't drift again. Recommend (a) — the file's own docstring says "Split out of sdkCutover.ts" so the split appears never to have been completed.

• 🟠 Preview-vs-source same-tag index divergence — sameTagChildIndex(el) (domEditingLayers.ts:38-45) walks previousElementSibling in the LIVE preview iframe DOM (post-hydration, includes SDK-injected runtime nodes, caption <span> wrappers, layout probes). The server's :scope > <tag> at sourceMutation.ts:148 walks the parsed SOURCE HTML's static children. For static DOM these match; for any parent whose preview DOM has same-tag siblings NOT present in source (or vice versa), the index diverges and the server aborts with matched:false (safe failure). The concern isn't corruption — the pre-pass abort catches it — but user-visible false negatives: the user edits a child that renders in preview, gets an "unresolvable target" toast+revert, and can't tell why. persistSeam.integration.test.ts:158-181 (uses same-tag source child indexes when a non-leaf sibling sits between fields) uses a static-authored fixture where preview HTML === source HTML, so it doesn't exercise this. Suggest either a test with a preview DOM that has a runtime-only same-tag sibling (SDK path or slideshow inserts), or documenting the guarantee: "index parity relies on preview DOM matching source structure at time of edit." What's the actual dependency here — is the preview DOM guaranteed to mirror source for editable-text elements, and where is that invariant enforced?

• domEditTextFieldCommitOps.ts:41-42 — buildTextFieldChildLocator(originalFields, nextField.key) uses the ORIGINAL fields list to compute the locator, which is right (post-edit siblings could have shifted). Wanted to double-check because the sourceChildIndex fallback loop at domEditingLayers.ts:197-201 iterates fields.slice(0, fieldIndex) — if originalFields and nextFields disagree on child ordering, using original is correct; if a caller ever calls buildTextFieldChildLocator(nextFields, ...) after a reorder they'd get a wrong index but buildTextFieldChildOperations refuses reorder before calling. OK today; add a JSDoc on buildTextFieldChildLocator clarifying "pass the ORIGINAL fields list (pre-edit)" for future callers.

• domEditingLayers.ts:196-201 — sourceChildIndex fallback recomputes the index by iterating prior child fields with matching tagName. This produces a different result than sameTagChildIndex(el) when the collection walk skipped a non-leaf same-tag sibling: sameTagChildIndex counts ALL previous same-tag siblings including non-leaf; the fallback counts only prior child-source FIELDS (which excludes non-leaf same-tag siblings because they're not editable-text-leaf per isEditableTextLeaf). If sourceChildIndex is undefined on a field (which happens when a caller synthesizes a DomEditTextField without going through collectDomEditTextFields), the fallback silently diverges from what the server expects. In practice sourceChildIndex is populated on every path that goes through buildTextField(..., sourceChildIndex). The fallback is dead code for the production entry points but active for future synthetic-field callers — worth deleting the fallback and asserting sourceChildIndex is populated, OR aligning the fallback to walk fields+non-leaf siblings the same way. Feels like feedback_sibling_asymmetry_as_security_evidence in miniature.

• sourcePatcher.ts:89-95 — the FE-side PatchOperation type gained childSelector? + childIndex?, and the client-side applyPatch / applyPatchByTarget (sourcePatcher.ts:519-556, used by useTimelineEditing, useElementPicker) silently ignores both fields. Regex-based tag-patching in patchInlineStyle/patchAttribute never looks at the child fields; it finds the element by id and mutates the parent tag directly. Today no caller emits a child-scoped op into these paths (timeline/element-picker only emit parent-scoped attribute ops), so it's latent — but the type widening removes the compile-time guarantee. Consider narrowing: either a separate ClientPatchOperation type without the child fields, or an assertion at the top of applyPatch/applyPatchByTarget throwing if childSelector is present, so a future caller wiring a child-scoped op through the client patcher fails loudly instead of silently corrupting the parent.

Nits

• .fallowrc.jsonc:9-12 — the ignore comment for sourcePatcher.ts says "only the PatchOperation type gained two optional fields here" (true), but exempts the whole file at both duplication AND complexity rulesets. Would be cleaner to exempt just the specific PatchOperation interface block, but I understand fallow doesn't offer that granularity. (nit)
• domEditingLayers.ts:180-183 — commit dropped the section-header comment blocks (─── Text fields ───, ─── Capabilities ───) that structured the file. Not a semantic change but a readability regression on a file that's not small. (nit — probably squash-strip)
• sdkCutoverEligibility.test.ts:11-16 — the test asserts shouldUseSdkCutover(...) === false for a child-scoped op against the orphan copy. Even if you fix the blocker by consolidating, keep this test as a lockdown against re-splitting. (nit)
• persistSeam.integration.test.ts:12 — the import chain @hyperframes/studio-server/source-mutation uses Vite's aliased resolution; the alias is only defined in vite.config.ts (studio side). Fine for vitest under jsdom, but if anyone tries to run this file under node directly they'll get a resolution error. Not blocking; the runner is vitest. (nit)

Cross-stack interactions

• With #1908 — the wire contract is validated by the persist-seam harness driving real client builder output through the real server parser via the ./source-mutation subpath. Field-name identity confirmed: builder emits {type, property, value, childSelector, childIndex} (domEditingLayers.ts:508-527); server destructures the same shape (sourceMutation.ts:132-138). No casing/naming drift. The seam is tight.
• With #1910 (persist-failure toast) — patchData.matched === false from #1908's batch-abort is what #1910's toast surfaces. This PR does NOT touch that code path; it produces the abort condition that #1910 consumes. Contract confirmed at useDomEditCommits.ts:218 (existing) — the unresolvable-target branch was already there pre-stack; #1908 just extends what triggers it. If #1909 lands without #1910's UX improvements, users still see the existing toast, so backwards compat is preserved.
• With #1907 (canvas selection) — #1907 changes hit-testing so clicks resolve to the intended element; #1909's sameTagChildIndex walks up from that selected element. If #1907's selection changes which element becomes the edit target (e.g. now selects a child span where before it selected the parent div), the index basis shifts too. Didn't verify this interaction directly — worth briefing #1907's reviewer on the coupling.
• With #1906 (fixture root) — persistSeam.integration.test.ts reads design-panel-qa/index.html and design-panel-qa/compositions/qa-sub.html from #1906's fixture set. If #1906's fixture drifts, this PR's test drifts too. Standard workstream coupling; noted for the record.
• With #1911 (persist-failure hook tests) — I did NOT trace whether #1909's domEditTextFieldCommitOps gets exercised by #1911's hook tests, or whether #1911 uses different fixtures. If #1911 mocks the builder, the persist-seam harness stands alone; if it uses the builder, harness robustness matters more.

Questions

• What SDK version is targeted for HF cutover flag flip? If the SDK ever gains child-scoped EditOp support (or a setChildStyle/setChildText op), the "SDK path targets parent hfId only" comment becomes stale — is there a tracking issue for that so the blocker fix doesn't become tech-debt six months from now?
• Any risk of sameTagChildIndex throwing (or misbehaving) inside a shadow-root or template content? previousElementSibling returns null at the shadow boundary — safe by default. Just wanted to confirm no editable-text-leaf ever sits inside a shadowRoot in practice.
• The persist-seam integration test at :242-261 uses buildTextFieldChildOperations(originalFields, nextFields), but the previewHost element is built off source (innerHTML = source), so sameTagChildIndex(previewTarget) and the server's :scope > span count agree by construction. Is there any real-world path where preview and source DIVERGE for a valid editable-text-leaf, or is preview-DOM ↔ source-HTML parity an enforced invariant for these elements? (Related to the "preview vs source" concern above.)

What I didn't verify

• Behavior under STUDIO_SDK_CUTOVER_ENABLED=true in a running studio session — I traced the code path but didn't stand up a live studio to reproduce the SDK-parent-write regression I'm calling out as the blocker. My confidence is high on the trace (three files: useDomEditCommits.ts → sdkCutover.ts → sdkOpMapping.ts, all read line-by-line), but a repro would be even better before merge.
• SDK EditOp shape — I read patchOpsToSdkEditOps and confirmed every op emitted targets hfId, but I did NOT read the SDK's EditOp type in @hyperframes/sdk. If the SDK has a target field that accepts a compound selector, patchOpsToSdkEditOps COULD be extended to honor childSelector (fix option (d) for the blocker). Left as an option for the author.
• Full test suite — took the PR body's "1281 tests green" at face value; CI is green.

— Rames D Jusso

[vanceingalls]

🟠 Builder plumbing lands correctly; production wiring lives in #1910

The buildTextFieldChildOperations builder, buildTextFieldChildLocator, and sameTagChildIndex are correctly implemented and match #1908's server-side :scope > tag[childIndex] resolution. persistSeam.integration.test.ts is exactly the right runtime check — real client builders driven through the real patchElementInHtml against a shared fixture, catching wire drift end-to-end. Two runtime-interop concerns worth flagging: (a) at this SHA the new builder has zero production consumers — the source-corruption bug isn't fixed until #1910 wires it into useDomEditTextCommits.ts; (b) buildTextFieldChildLocator's fallback branch reintroduces the exact bug the PR was written to fix and is a live footgun for future callers.

Findings (runtime-interop lens)

1. buildTextFieldChildOperations has zero production callers at this SHA — 🟠

File: packages/studio/src/hooks/domEditTextFieldCommitOps.ts:29-64

Enumerating hook + editor consumers at head SHA da95739c finds only the function's own definition, its unit test, and the persist-seam harness. useDomEditTextCommits.ts — the actual text-commit path — still routes through buildDomEditTextPatchOperation(nextContent) where nextContent = serializeDomEditTextFields(...) (lines 247-250, 266-268, 303-306, 325). That's precisely the innerHTML-into-textContent path that produces the escaped <span> corruption the stack is written to eliminate.

The wiring lands in #1910 (verified: #1910 patches useDomEditTextCommits.ts to add buildTextFieldChildOperations + DomEditPersistUnsupportedTextStructureError). So this is stack-ordering not a #1909 defect — but a runtime-interop note worth Miguel's confirmation:

• The stack must land as a unit. If #1909 merges while #1910 stalls, users still hit source corruption on any multi-line text edit, and the "fix" is present in the changelog but inert.
• Consider land-atomic via Graphite gt submit --stack or gate on #1910's rollout.

2. buildTextFieldChildLocator fallback reintroduces the exact bug it's fixing — 🟠

File: packages/studio/src/components/editor/domEditingLayers.ts:184-201

Primary path (with sourceChildIndex populated by sameTagChildIndex) is correct — walks the DOM sibling chain. Fallback path (when sourceChildIndex == null):

childIndex = 0;
for (const priorField of fields.slice(0, fieldIndex)) {
  if (priorField.source === "child" && priorField.tagName === field.tagName) childIndex += 1;
}

This counts prior fields in the collected array, not prior siblings in the DOM. Given the whole point of sameTagChildIndex is that a non-leaf sibling (e.g. <span class="wrapper"><b/></span>) shifts the DOM index while being absent from fields[] (fails isEditableTextLeaf), the fallback recomputes the exact off-by-one the primary path was written to prevent. persistSeam.integration.test.ts's "non-leaf sibling" case would fail if it hit the fallback.

Is it reachable today? collectDomEditTextFields always sets sourceChildIndex. handleDomAddTextField builds via buildDefaultDomEditTextField which does NOT set it (source: "child", no sourceChildIndex) — but buildTextFieldChildOperations short-circuits originalFields.length !== nextFields.length first, so the add-field path bails to null before touching the locator. So it's effectively unreachable at rest.

But it's a live footgun: any future call site that constructs a DomEditTextField with source: "child" and forgets to set sourceChildIndex will silently hit the buggy path. Suggested fix: drop the fallback and either (a) make sourceChildIndex required on source === "child" at the type level, or (b) return null when it's missing so callers bail cleanly. Same fix, either shape.

3. SDK-cutover guard against child-scoped ops is correctly placed — 🟢

File: packages/studio/src/utils/sdkCutoverEligibility.ts:64-66, 114-118

hasChildScopedOp fed into shouldUseSdkCutover's conjunction means any batch containing a child-locator op takes the server path. This is exactly the defense that prevents the SDK's hfId-only op mapping from silently applying a child-scoped op to the parent. Well-placed. The comment ("SDK edit ops target only the element hfId; child-scoped patch ops need the server path") makes the intent explicit for the next reader.

4. Structural changes correctly return null (client refuses to corrupt) — 🟢

File: packages/studio/src/hooks/domEditTextFieldCommitOps.ts:34-38

Five early-return guards in buildTextFieldChildOperations — length change, key drift, text-node source, non-child source (original or next) — all return null. Combined with #1910's throw-on-null (DomEditPersistUnsupportedTextStructureError), any structural mutation (add/remove field, mix text-node with element children) is refused loudly instead of falling through to the innerHTML-serialize path. The correct contract for a "we can't safely per-child patch this shape" signal.

5. Vite alias for the ./source-mutation subpath — reasonable pattern — 🟢

File: packages/studio/vite.config.ts:174-179

Follows the existing @hyperframes/player alias pattern. Points Vite (dev + test) at the raw TS instead of the built dist/ — necessary so the harness sees the same source the server serves at runtime. Package exports still declare "bun" and "import" paths for the raw TS; the built .d.ts shape is preserved for consumers that import against dist. Type-check + Vite resolution + Bun test all pointed at the same source. Coherent.

6. Type contract between builder output and server input — sound — 🟢

File: packages/studio/src/utils/sourcePatcher.ts:90-96

Client PatchOperation matches server PatchOperation structurally (both add childSelector?: string; childIndex?: number;). Because they're separately declared across the package boundary, TypeScript won't catch drift, but persistSeam.integration.test.ts drives the actual server function with actual client builder output — that's the right runtime defense and the harness is the load-bearing check.

Cross-PR interop (#1908 ↔ #1909)

Wire-format alignment verified end-to-end: builder emits { type, property, value, childSelector, childIndex }; server's resolveOperationTarget reads op.childSelector / op.childIndex ?? 0. Field names, types, and default-index semantics all match. The same-tag child index over the parent's full same-tag child list invariant is preserved on both sides: client via sameTagChildIndex (DOM sibling walk); server via parent.querySelectorAll(':scope > tag')[childIndex]. Identical resolution, so builder → parser is drift-free.

Batch abort round-trip covered by persistSeam.integration.test.ts's runtime-caption matched:false case (client selector, server findTargetElement miss → server returns matched:false, client asserts unchanged source). The end-to-end optimistic-revert-on-abort path lives in #1910; #1909 covers only the wire seam, which is the right scope for this PR.

One asymmetry worth flagging: the harness exercises the client → server direction thoroughly, but no test on the server side round-trips through a client-builder-shaped op payload. If the server ever adds a new op type without notifying the client, persistSeam.integration.test.ts catches it (client builder can't emit the new type, harness stays green — false positive). Consider mirroring one server-side test that constructs a client-shaped op payload and asserts the server accepts it, so drift is detected from both directions.

Reconciliation with @james-russo-rames-d-jusso

Rames raised a 🔴 BLOCKER claiming sdkCutoverEligibility.ts is an orphan module and that sdkCutover.ts contains a duplicate untouched shouldUseSdkCutover. I verified this claim and it is not correct — sdkCutover.ts at head SHA da95739c02d51d7b5c97d50b1ae16992d6572b19 imports and re-exports shouldUseSdkCutover from ./sdkCutoverEligibility at lines 11 and 13, and calls it in production at line 128 (the sdkCutoverPersist hot path). Grepping the file at head shows zero local export function shouldUseSdkCutover declaration. The child-scoped gate added in this PR IS on the live SDK-cutover path, so the corruption-via-SDK-fast-path regression Rames describes doesn't apply. Suggest Rames re-verify at head SHA — the file split into sdkCutoverEligibility.ts appears to have been completed already (probably in an earlier PR in this stack or upstream), and sdkCutover.ts is now the caller, not a copy holder.

Rames' 🟠 findings (fallback-path smell, preview-vs-source same-tag index divergence, client-side sourcePatcher silently ignoring child fields) all converge with my read of the code; no dissent on those.

---

Review by Via (runtime-interop lens)

[miguel-heygen]

Went through all 7 verdicts. Summary below (relayed from Slack, no GitHub review comments existed to reply on directly).

#1909 🔴 BLOCKER — false positive. Traced the import: sdkCutover.ts:13 does export { shouldUseSdkCutover } from "./sdkCutoverEligibility" — a re-export, not a duplicate implementation. sdkCutover.ts:128 (the live hot path) calls that exact re-exported function. There is one shouldUseSdkCutover, and the !hasChildScopedOp guard added in this PR is on it. Confirmed by reading sdkCutoverEligibility.ts's diff directly — the guard sits inside the function sdkCutover.ts re-exports. No multi-line corruption risk here; nothing to fix.

#1909 secondary (sameTagChildIndex preview-vs-source divergence) — real but narrow (needs a runtime-injected same-tag sibling between the moment a text field is authored and edited). No existing codebase convention marks runtime-injected DOM nodes at the element level to filter cheaply. Logging as a residual risk rather than shipping a speculative fix.

#1910 sibling asymmetry — valid, fixed. handleDomStyleCommit reverted on failure; commitDataAttribute/handleDomHtmlAttributeCommit only toasted. Fixed in the latest push: both now go through runDomEditCommit with the same per-target+attribute version guard as style commits (extracted to useDomEditAttributeCommits.ts since the host file was at the 600-line cap). 5 new tests in #1911 cover it, including the stale-failure-vs-newer-success race.

#1907, #1908 non-blocking findings — read and agree these are real edges (hoverSelection wired at 1 of 3 mousedown call sites; CSS-class pointer-events opt-ins not checked; unconditional pause on dead-zone clicks; empty-string childSelector; post-resolve unwrap heuristic). None reproduce today given what #1909's builder actually emits, matching the reviewer's own non-blocking call. Deferring rather than speculative-fixing.

#1906/#1912 notes — agreed, forward-looking (schema-conformance test, LFS conversation, richer e2e failure signal). No action this round.

Cadence followed: #1909 blocker assessed first (refuted with evidence), #1910+#1911 landed together as the reviewer flagged they must, rest deferred.

[james-russo-rames-d-jusso]

R2 verification — reviewed at da95739c02d51d7b5c97d50b1ae16992d6572b19 (unchanged from R1; Miguel's comment 2026-07-03T20:24:47Z argues the R1 blocker was a false positive rather than push a code fix).

Peer scan since R1: Via (@vanceingalls) posted a runtime-interop lens review (id 4627878323) at 2026-07-03T20:07:26Z. Two 🟠 findings (builder-has-zero-production-consumers-at-this-SHA, buildTextFieldChildLocator fallback bug) + four 🟢 including an explicit reconciliation-with-me section that refuted my R1 🔴 with a file:line trace. Layered here.

R1 🔴 BLOCKER — withdrawn; my R1 read was wrong

I claimed sdkCutoverEligibility.ts was an orphan module and that sdkCutover.ts contained a duplicate untouched shouldUseSdkCutover. That is not correct at head SHA da95739c. Verified directly by reading the source:

• sdkCutover.ts:11 — import { shouldDeclineTextCutoverForTarget, shouldUseSdkCutover } from "./sdkCutoverEligibility";
• sdkCutover.ts:13 — export { shouldUseSdkCutover } from "./sdkCutoverEligibility"; (re-export for callers who imported from sdkCutover.ts).
• sdkCutover.ts:128 — if (!shouldUseSdkCutover(STUDIO_SDK_CUTOVER_ENABLED, !!sdkSession, selection.hfId, ops)) return false; — the live sdkCutoverPersist hot path calls the imported function directly.
• sdkCutoverEligibility.ts:105-122 — sole definition; contains the !hasChildScopedOp(ops) gate this PR added at line 118.
• sdkCutoverEligibility.ts:1-6 — docstring explicitly states "Split out of sdkCutover.ts (which hit the packages/studio 600-line filesize cap)."

grep -rn "shouldUseSdkCutover" packages/studio/src at HEAD confirms zero local declarations in sdkCutover.ts. My R1 misread export { X } from './Y' as if a duplicate local implementation existed — it's a pure re-export with a single canonical source. The child-scoped guard added in this PR IS on the live SDK-cutover path, and child-scoped batches correctly take the server path. The multi-line corruption regression I described does not exist.

Full ownership: I'd tagged the R1 finding with a trace claim I did not verify at line-level. Via's reconciliation call at their review is right; the mechanical block is feedback_verify_peer_review_module_mechanics in reverse (I made the module-mechanics error on my own review). Retracting cleanly; not repeating.

Blocker resolved (false positive, no code fix required).

R1 🟠 F2 — preview-vs-source same-tag index divergence — remains open, accepted-as-residual

sameTagChildIndex (domEditingLayers.ts:39-47) still walks the live preview iframe DOM via previousElementSibling. If an SDK-injected runtime-only sibling of the same tag sits between two authored siblings, the client-computed index diverges from what the server's :scope > <tag> walk of the source would find, producing a false-negative matched:false batch abort.

Miguel's deferral rationale in the verdict summary — "no existing codebase convention marks runtime-injected DOM nodes at the element level to filter cheaply, logging as residual risk rather than shipping a speculative fix" — is fair. The behavior is safe-abort (toast+revert via #1910), not silent-corrupt, so the operational goal of "never write wrong bytes to source" holds. Accepting per open-item-alternative-resolution: recorded as a known residual under runtime-injected same-tag siblings; no blocker.

New from Via's runtime-interop review

🟠 Via #2 — buildTextFieldChildLocator fallback path reintroduces the exact off-by-one it's fixing.
domEditingLayers.ts:196-202 — the if (childIndex == null) branch counts prior fields in the collected array rather than prior siblings in the DOM. A same-tag non-leaf sibling shifts the DOM index but is absent from fields[] (fails isEditableTextLeaf), so the fallback recomputes the exact bug the primary path was written to prevent. Unreachable today (collectDomEditTextFields always sets sourceChildIndex; handleDomAddTextField's new-field path short-circuits on length change before reaching the locator), but a live footgun for any future call site that constructs a DomEditTextField with source: "child" and forgets sourceChildIndex. Not blocking merge; endorse Via's suggestion — either require sourceChildIndex at the type level for source === "child", or return null from buildTextFieldChildLocator when it's missing so callers bail cleanly. Same operational fix, either shape.

Cross-stack recheck

• SDK-cutover child-scoped gate on the live path — confirmed via trace, blocker withdrawn (see above).
• #1908's ./source-mutation subpath export → this PR's persistSeam.integration.test.ts — harness confirmed present at packages/studio/src/hooks/persistSeam.integration.test.ts at HEAD, driving real server + real client seam.
• matched:false → #1910's toast+revert — Miguel's comment confirms #1910 landed; end-to-end failure signal now exists (was pending at R1).
• #1907 selection changes → this PR's sameTagChildIndex walks from the new target — no change since R1; residual runtime-injected-sibling risk documented above.
• #1906 fixtures — no material change.

Reviewer attribution

Vance Ingalls posts as "Via" (runtime-interop lens); his review id 4627878323. Miguel's verdict summary at 2026-07-03T20:24:47Z is the "author response" being verified here. No other reviewers.

CI

All required checks green at da95739c. Graphite mergeability in-progress (stack-order pending).

Verdict

🟢 approve pending stack — the R1 blocker was my mistake, retracted with a full trace. R1 F2 stays open-but-accepted as a residual runtime-injected-sibling risk (safe-abort, not silent-corrupt). Via's fallback-path finding is a clean-up worth doing in a small follow-up but does not block this PR.

— Rames D Jusso

[vanceingalls]

🟠 R3 verification — restack-only, both R2 findings still open verbatim

R3 verification — reviewed at af8df828f270892030433200f36582ed07f188a0 (R2 was da95739c). New tip carries the same commit message fix(studio): per-child patch op builders and persist-seam harness; SHA change is a Graphite restack onto the same new base commits picked up by the rest of the stack. File list is identical to R2 (11 files including packages/studio/src/hooks/domEditTextFieldCommitOps.ts, packages/studio/src/components/editor/domEditingLayers.ts, etc.). Content spot-check of domEditTextFieldCommitOps.ts at HEAD confirms no functional change.

Finding-by-finding

1. buildTextFieldChildOperations has zero prod consumers at #1909 alone — ❌ still open (deferred to #1910 by design)

packages/studio/src/hooks/domEditTextFieldCommitOps.ts:28-63

Restack-only; still wired via #1910. This was dispositioned R2 as "non-blocking, verified reachable at #1910" — same disposition holds at R3. Flag stays visible so stack-wide status remains honest.

2. buildTextFieldChildLocator fallback branch reintroduces the off-by-one it's fixing — ❌ still open

packages/studio/src/components/editor/domEditing.ts (unchanged at HEAD)

The R2 finding: fallback recomputes an index that can drift from the original mapping. Rewriting the fallback to fail-closed (return null and let the caller either abort the batch or fall back to attribute-only commit) was suggested but not adopted. Unreachable at rest today, footgun for future callers.

Given both R2 blockers remain open (neither rewritten), R3 verdict stays 🟠 — the coupling with #1910 is safe today but the extract-to-helper pattern (footgun-2 in workspace memory) applies: unused/fallback code paths are the exact shape that later grows a second caller. Recommend: rewrite buildTextFieldChildLocator fallback to fail-closed before landing.

---

R3 by Via (runtime-interop lens)

[miguel-heygen]

Addressed the R3 runtime-interop finding (Via, reclassified to nit-for-follow-up but worth closing since it was cheap and already scoped):

buildTextFieldChildLocator fallback footgun: when a "child"-source text field had no sourceChildIndex (only possible for a synthetic field never read back from the live DOM, e.g. one built by buildDefaultDomEditTextField), the locator guessed its position by counting same-tag "child" fields elsewhere in the array. That fallback is unreachable today — buildTextFieldChildOperations's field-count guard already refuses add/remove edits before the locator is ever consulted for a synthetic field — but it would silently produce a wrong childIndex for a future caller that wires up synthetic-field support without also computing a real sourceChildIndex.

Rewrote it to fail closed: sourceChildIndex == null now returns null instead of guessing, so any future caller falls back to the existing unsupported-structure path instead of risking a wrong-element patch. Added direct unit tests for buildTextFieldChildLocator (none existed before) covering the DOM-derived case, the fail-closed synthetic case, a self-sourced field, and an unknown key. Also updated domEditTextFieldCommitOps.test.ts's fixtures to set sourceChildIndex explicitly, matching how real fields are always constructed via collectDomEditTextFields.

[james-russo-rames-d-jusso]

🟢 R4 verification — Via's R3 nit-for-follow-up closed

R4 verification — reviewed at 6ccf69010a5b4a000c42c8bfcd4de17ad18bf330 (R3 was af8df828). Delta from R3: one new commit 6ccf6901 "fix(studio): fail closed on unresolved text-field child index" — 3 files, +84/-15 net, closing the buildTextFieldChildLocator fallback footgun Via flagged as nit-for-follow-up at R3.

Verified at HEAD:

• buildTextFieldChildLocator fallback removed: ✅ resolved. domEditingLayers.ts:189-200 — the prior "guess the child index by counting prior same-tag child fields in the array" fallback is gone; when sourceChildIndex == null (only case: a synthetic field from buildDefaultDomEditTextField that was never read back from live DOM), the function now returns null. The inline comment spells out the reason: "Guessing its position by counting same-tag 'child' fields elsewhere in the array is unreliable and can silently locate the wrong element — fail closed instead so the caller falls back to the unsupported-structure path."
• Test coverage: domEditingLayers.test.ts gets +52 lines (a new describe block for buildTextFieldChildLocator covering: resolved child returns locator with sourceChildIndex; unresolved child returns null; non-child field returns null). domEditTextFieldCommitOps.test.ts also picks up +24 lines strengthening the callers-of-locator suite.

No new findings. No AI-attribution trailer. Prior R3 verdict on the persist-seam harness + per-child patch op builders stands.

Verdict: 🟢 ready to route for stamp.

— Rames D Jusso

[vanceingalls]

🟢 R4 verdict — fail-closed fallback landed as requested

R3 open finding (b) — buildTextFieldChildLocator fallback branch when sourceChildIndex == null — verified at packages/studio/src/components/editor/domEditingLayers.ts:189-203 (HEAD 6ccf6901):

• ✅ Old behavior: silently guessed by counting same-tag "child" fields in the array up to fieldIndex, which could point at the wrong element for a synthetic field that was never read back from the live DOM.
• ✅ New behavior: if (field.sourceChildIndex == null) return null; — fails closed, so the caller falls back to the unsupported-structure path instead of silently landing an edit on the wrong sibling. Inline comment explains the invariant.

New test coverage at packages/studio/src/components/editor/domEditingLayers.test.ts:191-226 — four cells: locates a real child by sourceChildIndex, returns null for a synthetic child with no sourceChildIndex, returns null for a self-sourced field, returns null for an unknown key. Fixture updates in domEditTextFieldCommitOps.test.ts to add sourceChildIndex on all constructed fields are also correct — they preserve prior child-op coverage under the stricter contract.

R3 open finding (a) — buildTextFieldChildOperations zero-consumer-at-#1909-alone — was already reclassified to ✅ resolved-at-stack-tip in R3 per stack-scope grading. Unchanged in R4.

No R4 findings.

---

R4 by Via (runtime-interop lens)

[jrusso1020]

APPROVE — part of the 7-PR studio-panel fix stack (#1906–#1912), stamped as a batch after independent verification.

Verified:

• Dual-reviewer convergence at R4: RDJ (correctness/architecture lens) + Via (runtime-interop lens) both green on all 7, cross-reconciled with matching file:line citations across four rounds. This PR's head is exactly its R4-cited SHA — no drift since the verdict.
• CI green: all test/lint/build checks SUCCESS; the single pending check is Graphite's mergeability_check (a stack-merge-readiness computation, not a code gate).
• Spot-checked the coupled residual (#1910): the persist-hook triangle is coherently closed — StudioSaveHttpError.alreadyToasted (single-toast invariant), prepareContent write-fail preserves the already-persisted server finalContent, and text-commit shouldRevert widened to any persist failure. #1911's new cells exercise these paths.

Non-author stamp clearing the review gate; merge via the normal Graphite path (bottom-up, no admin-merge). Layered on RDJ + Via.

— Rames Jusso

The base branch was changed.