fix(studio-server): address PR #2097 review feedback on relative-timing resolver

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.
This commit is contained in:
Vance Ingalls
2026-07-09 11:27:53 -07:00
parent fbd21d5709
commit e4c4d2e15d
2 changed files with 23 additions and 0 deletions
@@ -293,6 +293,18 @@ describe("T10 — PreviewAdapter contract (spec for R7)", () => {
expect(timings["hf-orphan"].start).toBeUndefined(); 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", () => { 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-a", "data-start": "hf-b", "data-duration": "2" });
make("div", { "data-hf-id": "hf-b", "data-start": "hf-a", "data-duration": "3" }); make("div", { "data-hf-id": "hf-b", "data-start": "hf-a", "data-duration": "3" });
@@ -147,6 +147,12 @@ export function createPreviewAdapter(
// has no data-end) — this function never read data-duration before either, // 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 // so a reference to a duration-authored (not end-authored) clip used to be
// unresolvable regardless of the parseFloat bug. // 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<Element, number | undefined>(); const startCache = new Map<Element, number | undefined>();
const visiting = new Set<Element>(); const visiting = new Set<Element>();
@@ -184,6 +190,11 @@ export function createPreviewAdapter(
} else if (expr?.kind === "absolute") { } else if (expr?.kind === "absolute") {
resolved = expr.value; resolved = expr.value;
} else { } 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; const sv = startStr !== null ? parseFloat(startStr) : NaN;
resolved = Number.isFinite(sv) ? sv : undefined; resolved = Number.isFinite(sv) ? sv : undefined;
} }