From e4c4d2e15d76f912699df8551e4d1639754154a2 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 9 Jul 2026 10:57:00 -0700 Subject: [PATCH] fix(studio-server): address PR #2097 review feedback on relative-timing resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the shared-pattern context (3rd copy of "resolve relative data-start", after runtime startResolver.ts and the SDK's own getElementTimings) and explains when the raw parseFloat fallback in resolveStart's else branch can actually fire (a malformed grammar string with a leading number). Adds a test pinning the "reference target exists but its own timing is unresolvable" branch, which existing tests didn't cover (only "target doesn't exist" was tested). Cross-checked the negative-offset clamp concern raised in review: the SDK's own resolveReferenceStart (session.ts) also clamps to Math.max(0, ...), so this stays consistent with its sibling — no code change needed there. --- .../studio-server/src/helpers/previewAdapter.test.ts | 12 ++++++++++++ packages/studio-server/src/helpers/previewAdapter.ts | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/studio-server/src/helpers/previewAdapter.test.ts b/packages/studio-server/src/helpers/previewAdapter.test.ts index 5881449bc..6ef7d8814 100644 --- a/packages/studio-server/src/helpers/previewAdapter.test.ts +++ b/packages/studio-server/src/helpers/previewAdapter.test.ts @@ -293,6 +293,18 @@ describe("T10 — PreviewAdapter contract (spec for R7)", () => { expect(timings["hf-orphan"].start).toBeUndefined(); }); + it("returns undefined start when the reference target exists but its own timing is unresolvable", () => { + make("div", { "data-hf-id": "hf-untimed" }); // no data-end, no data-duration + make("div", { + "data-hf-id": "hf-outro", + "data-start": "hf-untimed + 2", + "data-duration": "1", + }); + const adapter = adapterWith(() => null); + const timings = adapter.getElementTimings(); + expect(timings["hf-outro"].start).toBeUndefined(); + }); + it("terminates (not an infinite loop) on a mutual A <-> B reference cycle", () => { make("div", { "data-hf-id": "hf-a", "data-start": "hf-b", "data-duration": "2" }); make("div", { "data-hf-id": "hf-b", "data-start": "hf-a", "data-duration": "3" }); diff --git a/packages/studio-server/src/helpers/previewAdapter.ts b/packages/studio-server/src/helpers/previewAdapter.ts index 4278710b6..e576a524c 100644 --- a/packages/studio-server/src/helpers/previewAdapter.ts +++ b/packages/studio-server/src/helpers/previewAdapter.ts @@ -147,6 +147,12 @@ export function createPreviewAdapter( // has no data-end) — this function never read data-duration before either, // so a reference to a duration-authored (not end-authored) clip used to be // unresolvable regardless of the parseFloat bug. + // + // This is the third copy of "resolve relative data-start" (runtime + // startResolver.ts; the SDK's own getElementTimings in session.ts; this + // one). The runtime version is substantially more complex (host offsets, + // media, live timelines) so a shared extraction isn't a straightforward + // win — this one and the SDK's stay hand-kept in sync instead. const startCache = new Map(); const visiting = new Set(); @@ -184,6 +190,11 @@ export function createPreviewAdapter( } else if (expr?.kind === "absolute") { resolved = expr.value; } else { + // parseStartExpression returns null for empty/absent data-start, and + // also for a malformed grammar string (e.g. "3 abc" — a leading + // number followed by content the reference regex rejects). The + // parseFloat below only ever succeeds on that second, malformed + // case (a clean number or a clean reference already matched above). const sv = startStr !== null ? parseFloat(startStr) : NaN; resolved = Number.isFinite(sv) ? sv : undefined; }