From ba0d6406d23d11e12305ba79834a69ae672b8432 Mon Sep 17 00:00:00 2001
From: Miguel Angel Simon Sierra <miguel.sierra@heygen.com>
Date: Tue, 28 Jul 2026 21:21:27 +0200
Subject: [PATCH] fix(studio): scope lane ids per timeline and stop inventing a
 track row

Two latent defects in the announcement path this branch adds.

- lanesId was keyed by render row alone, so a second TimelineLanes on the
  page (a mini-timeline beside the main one) would mint the same
  timeline-lanes-track-0 and every caret's aria-controls would resolve to
  whichever instance mounted first. The prefix now comes from useId, with the
  colons stripped so the id stays a legal CSS selector.
- trackDisplayNumber returned trackOrder.length + 1 for a key it could not
  find, which is indistinguishable from a real row: the label announced a row
  the user can see is wrong and nothing upstream could tell it had guessed. It
  returns null now, and trackDisplaySuffix drops the number from the label
  rather than inventing one.
---
 .../src/hooks/timelineTrackVisibility.ts      | 12 +++++--
 .../player/components/TimelineLanes.test.tsx  | 24 +++++++++++++
 .../src/player/components/TimelineLanes.tsx   | 16 +++++++--
 .../player/components/TimelineTrackHeader.tsx | 11 +++---
 .../components/timelineTrackDisplay.test.ts   | 35 +++++++++++++++++++
 .../player/components/timelineTrackDisplay.ts | 22 +++++++++---
 6 files changed, 106 insertions(+), 14 deletions(-)
 create mode 100644 packages/studio/src/player/components/timelineTrackDisplay.test.ts

diff --git a/packages/studio/src/hooks/timelineTrackVisibility.ts b/packages/studio/src/hooks/timelineTrackVisibility.ts
index 634ea6ee6..7860ad38b 100644
--- a/packages/studio/src/hooks/timelineTrackVisibility.ts
+++ b/packages/studio/src/hooks/timelineTrackVisibility.ts
@@ -1,7 +1,11 @@
 import { useCallback } from "react";
 import { usePlayerStore, type TimelineElement } from "../player";
 import { useExpandedTimelineElements } from "../player/hooks/useExpandedTimelineElements";
-import { timelineTrackOrder, trackDisplayNumber } from "../player/components/timelineTrackDisplay";
+import {
+  timelineTrackOrder,
+  trackDisplayNumber,
+  trackDisplaySuffix,
+} from "../player/components/timelineTrackDisplay";
 import { saveProjectFilesWithHistory } from "../utils/studioFileHistory";
 import { readTagSnippetByTarget, type PatchOperation } from "../utils/sourcePatcher";
 import {
@@ -211,13 +215,15 @@ export async function toggleTimelineTrackHidden({
 }: ToggleTimelineTrackHiddenInput): Promise<string[]> {
   // `track` is the fractional sort key the callback needs; the history entry is
   // read by a human, so it gets the display row instead.
-  const displayNumber = trackDisplayNumber(timelineTrackOrder(timelineElements), track);
+  const suffix = trackDisplaySuffix(
+    trackDisplayNumber(timelineTrackOrder(timelineElements), track),
+  );
   return setElementsHidden({
     projectId,
     activeCompPath,
     elements: timelineElements.filter((element) => element.track === track),
     hidden,
-    label: hidden ? `Hide track ${displayNumber}` : `Show track ${displayNumber}`,
+    label: hidden ? `Hide track${suffix}` : `Show track${suffix}`,
     previewIframe,
     writeProjectFile,
     recordEdit,
diff --git a/packages/studio/src/player/components/TimelineLanes.test.tsx b/packages/studio/src/player/components/TimelineLanes.test.tsx
index eecc98f89..248b1675a 100644
--- a/packages/studio/src/player/components/TimelineLanes.test.tsx
+++ b/packages/studio/src/player/components/TimelineLanes.test.tsx
@@ -215,6 +215,30 @@ describe("TimelineLanes disclosure target", () => {
     act(() => view.root.unmount());
   });
 
+  // Two timelines on one page (a mini-timeline in a modal beside the main one)
+  // both minted `timeline-lanes-track-0`, so every caret's aria-controls
+  // resolved to whichever instance mounted first.
+  it("mints lane ids that do not collide with a second TimelineLanes on the page", () => {
+    const first = renderLanes({ animations: ANIMATIONS, expandedClipIds: ["clip-a"] });
+    const second = renderLanes({ animations: ANIMATIONS, expandedClipIds: ["clip-a"] });
+
+    const idsFor = (host: HTMLElement) =>
+      Array.from(host.querySelectorAll("button[aria-controls]")).map((caret) =>
+        caret.getAttribute("aria-controls"),
+      );
+    const firstIds = idsFor(first.host);
+    const secondIds = idsFor(second.host);
+
+    expect(firstIds.length).toBeGreaterThan(0);
+    expect(firstIds.some((id) => secondIds.includes(id))).toBe(false);
+    // Still a legal CSS id selector: the aria-controls lookups above use `#id`.
+    for (const id of [...firstIds, ...secondIds]) {
+      expect(id).toMatch(/^[A-Za-z][\w-]*$/);
+    }
+    act(() => first.root.unmount());
+    act(() => second.root.unmount());
+  });
+
   // The passenger branch wraps [clip, lanes] in a transformed div that re-renders
   // on every pointer move. An unstable key there remounts the lanes and drops the
   // in-flight drag.
diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx
index 32e805631..58d857762 100644
--- a/packages/studio/src/player/components/TimelineLanes.tsx
+++ b/packages/studio/src/player/components/TimelineLanes.tsx
@@ -1,10 +1,11 @@
+import { useId } from "react";
 import { BeatStrip, BeatBackgroundLines } from "./BeatStrip";
 import { TimelineClip } from "./TimelineClip";
 import { TimelineClipDiamonds } from "./TimelineClipDiamonds";
 import { TimelinePropertyLanes } from "./TimelinePropertyLanes";
 import { TimelineTrackHeader } from "./TimelineTrackHeader";
 import { resolveTrackKeyframeClip } from "./useTimelineTrackLayout";
-import { trackDisplayNumber } from "./timelineTrackDisplay";
+import { trackDisplayNumber, trackDisplaySuffix } from "./timelineTrackDisplay";
 import { clipTimingStart } from "../../hooks/gsapShared";
 import { getTimelineEditCapabilities, resolveBlockedTimelineEditIntent } from "./timelineEditing";
 import { CLIP_Y, CLIP_HANDLE_W, TRACK_H, getTimelineRowHeight } from "./timelineLayout";
@@ -89,6 +90,12 @@ export function TimelineLanes({
   onRazorSplit,
   onRazorSplitAll,
 }: TimelineLanesProps) {
+  // Per-INSTANCE, so two timelines on one page (a mini-timeline in a modal
+  // beside the main one) cannot both mint `...-track-0` and have every caret's
+  // aria-controls resolve to whichever mounted first. React's useId embeds
+  // colons, which are legal in an id and in aria-controls but need escaping in
+  // a CSS `#id` selector, so they come out here and the prefix stays plain.
+  const lanesIdPrefix = `timeline-lanes${useId().replaceAll(":", "")}`;
   const expandedClipIds = usePlayerStore((s) => s.expandedClipIds);
   const toggleClipExpanded = usePlayerStore((s) => s.toggleClipExpanded);
   const toggleClipExpandedTracked = (key: string) => {
@@ -137,7 +144,7 @@ export function TimelineLanes({
           // the disclosure: the caret in the sticky header and the diamond lanes
           // on the canvas. Keyed by display row, not by `trackNum`, which is a
           // fractional sort key and would mint ids like `...-0.16666666666666666`.
-          const lanesId = `timeline-lanes-track-${row}`;
+          const lanesId = `${lanesIdPrefix}-track-${row}`;
           return (
             <div
               key={trackNum}
@@ -154,7 +161,10 @@ export function TimelineLanes({
                 // key, so it stays out of every label and in every callback.
                 trackDisplayNumber={displayNumber}
                 trackLabel={
-                  els[0]?.label ?? els[0]?.domId ?? els[0]?.id ?? `Track ${displayNumber}`
+                  els[0]?.label ??
+                  els[0]?.domId ??
+                  els[0]?.id ??
+                  `Track${trackDisplaySuffix(displayNumber)}`
                 }
                 lanesId={lanesId}
                 contentOrigin={contentOrigin}
diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx
index d8a00e24f..f3cc603ff 100644
--- a/packages/studio/src/player/components/TimelineTrackHeader.tsx
+++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx
@@ -15,6 +15,7 @@ import {
   type TimelinePropertyLane,
 } from "./trackHeaderLaneState";
 import { valueReadout } from "./trackHeaderLaneValues";
+import { trackDisplaySuffix } from "./timelineTrackDisplay";
 
 interface TimelineTrackHeaderProps {
   /** The track's real key: a FRACTIONAL z-order sort value. Routes callbacks;
@@ -22,8 +23,9 @@ interface TimelineTrackHeaderProps {
   trackNumber: number;
   /** The track's 1-based position in the rendered order: the only number safe
    *  to put in a label. Announcing `trackNumber` read out "track
-   *  0.16666666666666666". */
-  trackDisplayNumber: number;
+   *  0.16666666666666666". Null when the key has no row, which drops the number
+   *  from the label rather than inventing one (see trackDisplayNumber). */
+  trackDisplayNumber: number | null;
   trackLabel: string;
   /** Id of the canvas-side lanes element the disclosure caret expands. Minted by
    *  TimelineLanes, which is the one place that sees both subtrees. */
@@ -55,14 +57,15 @@ function VisibilityButton({
 }: {
   hidden: boolean;
   trackNumber: number;
-  trackDisplayNumber: number;
+  trackDisplayNumber: number | null;
   visible: boolean;
   onToggle: TimelineEditCallbacks["onToggleTrackHidden"];
 }) {
   if (!visible) return <span aria-hidden="true" className="h-6 w-6 shrink-0" />;
   // Display number in the text, real key in the callback. The two must not be
   // conflated in either direction.
-  const label = hidden ? `Show track ${trackDisplayNumber}` : `Hide track ${trackDisplayNumber}`;
+  const suffix = trackDisplaySuffix(trackDisplayNumber);
+  const label = hidden ? `Show track${suffix}` : `Hide track${suffix}`;
   return (
     <button
       type="button"
diff --git a/packages/studio/src/player/components/timelineTrackDisplay.test.ts b/packages/studio/src/player/components/timelineTrackDisplay.test.ts
new file mode 100644
index 000000000..7c7fe3c37
--- /dev/null
+++ b/packages/studio/src/player/components/timelineTrackDisplay.test.ts
@@ -0,0 +1,35 @@
+import { describe, expect, it } from "vitest";
+import { timelineTrackOrder, trackDisplayNumber, trackDisplaySuffix } from "./timelineTrackDisplay";
+
+describe("timelineTrackOrder", () => {
+  it("is the ascending distinct key order, fractional sub-comp keys included", () => {
+    const elements = [{ track: 1 }, { track: 0 }, { track: 1 / 6 }, { track: 1 }];
+
+    expect(timelineTrackOrder(elements)).toEqual([0, 1 / 6, 1]);
+  });
+});
+
+describe("trackDisplayNumber", () => {
+  it("is the key's 1-based row, not the fractional key itself", () => {
+    const order = timelineTrackOrder([{ track: 0 }, { track: 1 / 6 }, { track: 1 }]);
+
+    expect(trackDisplayNumber(order, 1 / 6)).toBe(2);
+    expect(trackDisplayNumber(order, 1)).toBe(3);
+  });
+
+  it("is null for a key with no row rather than a plausible-looking one", () => {
+    // The old end-row fallback announced "track 4" for a track the user cannot
+    // see at row 4, and nothing upstream could tell that apart from a real row.
+    expect(trackDisplayNumber([0, 1, 2], 7)).toBeNull();
+  });
+});
+
+describe("trackDisplaySuffix", () => {
+  it("names the row when there is one", () => {
+    expect(`Hide track${trackDisplaySuffix(3)}`).toBe("Hide track 3");
+  });
+
+  it("drops the number entirely when there is no row", () => {
+    expect(`Hide track${trackDisplaySuffix(null)}`).toBe("Hide track");
+  });
+});
diff --git a/packages/studio/src/player/components/timelineTrackDisplay.ts b/packages/studio/src/player/components/timelineTrackDisplay.ts
index 4bf059e93..5e1af3f6c 100644
--- a/packages/studio/src/player/components/timelineTrackDisplay.ts
+++ b/packages/studio/src/player/components/timelineTrackDisplay.ts
@@ -15,10 +15,24 @@ export function timelineTrackOrder(elements: readonly { track: number }[]): numb
 }
 
 /**
- * A track key's 1-based display row. A key not in `trackOrder` (a drag preview
- * onto a brand-new track, say) reads as the row it would land on at the end.
+ * A track key's 1-based display row, or null when the key is not in
+ * `trackOrder` at all.
+ *
+ * Both callers build `trackOrder` from the same elements the key came from, so
+ * null is unreachable by construction today. It is null rather than a number
+ * because the only numbers available to return (the end row, the last row) are
+ * indistinguishable from a real answer: a label would announce a row the user
+ * can see is wrong, and nothing upstream would ever learn it had guessed.
  */
-export function trackDisplayNumber(trackOrder: readonly number[], track: number): number {
+export function trackDisplayNumber(trackOrder: readonly number[], track: number): number | null {
   const row = trackOrder.indexOf(track);
-  return row < 0 ? trackOrder.length + 1 : row + 1;
+  return row < 0 ? null : row + 1;
+}
+
+/**
+ * The `" 3"` in `Hide track 3`, empty when there is no display row to name.
+ * Announcing "Hide track" is thin; announcing an invented row is wrong.
+ */
+export function trackDisplaySuffix(displayNumber: number | null): string {
+  return displayNumber === null ? "" : ` ${displayNumber}`;
 }