fix(studio): give the group row's caret the panel's glyph and size back

[vanceingalls]

Stacked on #3414. Part of restoring fixes stranded in the unreviewed #3363 — see #3413 for the full context.

What

The timeline group row was the only disclosure caret in the studio still drawn as a rotated, 11px, non-mono glyph. Both of the property panel's carets swap between ▸ and ▾ in font-mono:

• propertyPanelFxPresetRun.tsx (hf-fx-preset-run-caret) — collapsed ? "▸" : "▾"
• propertyPanelFxNodeOpenBody.tsx — details ? "▾" : "▸"

So the same affordance rendered smaller, and differently, on the row than in the panel it opens. Now mono, a size up, and swapped rather than rotated — a rotated ▸ also sits off-centre in its box because the glyph is not square.

Note the size is the value the original fix chose; hf-fx-preset-run-caret has no CSS rule of its own (it inherits), so "13px matches the panel exactly" is not something I measured — the mono + swap consistency is.

Tests

Three, mounting the header — there was no test file for this component at all, which is how it silently reverted:

• the glyph swaps rather than rotating
• no rotate transform on the glyph
• the mono class is present and the smaller text-[11px] is gone

Verified all three fail against the previous caret.

Checks

tsc --noEmit clean · new tests 3/3 · oxlint / oxfmt clean · fallow audit no issues · all touched files well under the studio 600-LOC limit.

🤖 Generated with Claude Code

[jrusso1020]

First review, no prior reviews or comments. Read both files plus the two cited panel carets at 26f3bf9835.

Strengths

The comparison targets are exactly as described. propertyPanelFxPresetRun.tsx:165-166 is hf-fx-preset-run-caret, font-mono, collapsed ? "▸" : "▾"; propertyPanelFxNodeOpenBody.tsx:244 is details ? "▾" : "▸" under a font-mono parent. Swap-rather-than-rotate is the right call and the reasoning behind it is correct — ▸ is not square, so rotating it shifts the optical centre inside a fixed 24px box.

The body declines a claim it cannot back, and it is right to. hf-fx-preset-run-caret genuinely has no CSS rule of its own — the only two references in the tree are the className itself and this PR's new comment — and its parent hf-fx-preset-run-label (:150) sets no size either, so that caret's rendered size really is unmeasured. Saying so beats asserting a match.

The new test's caret selector is unambiguous, which is easy to get wrong in a header with four buttons: only the disclosure button's aria-label contains "tracks" (TimelineGroupHeader.tsx:78); the lane toggle says "lanes" (:161).

Nits

The new code comment claims more than the body does, and the comment is the part that stays in the tree. :79-81 says "matching the property panel's disclosure carets (hf-fx-preset-run-caret, propertyPanelFxNodeOpenBody) — the same affordance should not be smaller here than it is there." But propertyPanelFxNodeOpenBody's caret sits under text-[9px] (:240), so at text-[13px] this one is now materially larger than that one, and "should not be smaller" is inverted for it. The body is careful about precisely this point; the comment should carry the same care. Narrowing it to the two things that are true — mono, and swapped rather than rotated — costs nothing and keeps the next reader from re-deriving a size match that was never measured.

The third test's name outruns its assertions. it("matches the property panel's carets: mono, and not the smaller 11px") reads only this component's own className: font-mono present, text-[13px] present, text-[11px] absent. Nothing in it looks at either panel caret, so if the panel's carets move, the test stays green while the invariant in its name is broken. Either rename it to what it actually pins, or assert against the panel caret if the cross-component match is the thing worth guarding. This matters a little more than usual given the file's stated purpose is to stop a silent revert: a test whose name overstates its reach is how the next person concludes the invariant is covered when it is not.

Important

This rung ran no test job, so the three new tests have never executed in CI. The full check list is 13 entries with no Test, Build, Typecheck, Lint or Format; #3413, which targets main, gets 45 including all of those. (regression reads pass while regression-shards reads skipping — the aggregate is passing over shards that never ran.) That is the stack's base and path gating rather than anything you did, but it lands oddly here specifically: TimelineGroupHeader.test.tsx is a brand-new file whose entire reason for existing is that this fix reverted once with no test watching it, and right now it is green only on your machine. Worth getting it to run once on a main-targeting rung before this lands.

Verdict: COMMENT
Reasoning: The change is small and correct and its comparison targets check out, with two claims — one in a comment, one in a test name — that reach past what was actually verified, plus the CI gap that leaves the new tests unexecuted.

— Rames Jusso

[miguel-heygen]

Strengths: TimelineGroupHeader.tsx:77-94 replaces the transform with explicit collapsed/expanded glyphs, preserves the existing accessible button state, and inherits the mono sizing at the actual disclosure control. TimelineGroupHeader.test.tsx:57-72 pins the glyph swap, absence of rotation, and typography regression independently.

Stack note: This approval is the code verdict for exact head 26f3bf98; #3413/#3414 carry separate blockers and the bottom rung carries the full stack gate.

Verdict: APPROVE
Reasoning: The caret restoration is scoped, accessible, and directly regression-tested with no separate correctness issue found.

— Magi

[vanceingalls]

Both nits fixed at 27b1faa0 — no behaviour change.

The comment. It claimed the size matched the panel's carets and that "the same affordance should not be smaller here than it is there", which is inverted for one of the two: propertyPanelFxNodeOpenBody.tsx:240 sits under text-[9px], so at 13px this one is materially larger, and hf-fx-preset-run-caret has no size rule at all. Narrowed to what holds — mono, and the size being this row's own call with no measured target to match:

// Mono, like both of the property panel's disclosure carets. The size is
// this row's own call and not a match to either of them: the node-body
// caret sits under `text-[9px]`, and `hf-fx-preset-run-caret` has no
// size rule at all, so there is no measured target to match.

The test name. Renamed to what it actually pins — keeps the caret mono at 13px, not back down at the 11px the row inherits — with a line saying the panel carets are not read here and no cross-component match is claimed. Left as a same-component assertion rather than reaching into the panel: the invariant worth guarding against the revert is this row not silently going back to the inherited 11px, and asserting against a caret whose size is unmeasured would pin nothing.

3 tests pass; full studio suite 4378 passing on this tip.

The CI gap is real and unchanged — this rung is base/path-gated, #3413 carries the main-targeting jobs.

[miguel-heygen]

Strengths: TimelineGroupHeader.tsx:77-94 still carries the correct behavioral restoration, and the follow-up now states only what is measured: mono, explicit 13px, and swapped glyphs rather than a cross-component size match. TimelineGroupHeader.test.tsx:67-74 was renamed to the same honest invariant.

I closed the stacked-CI gap locally at exact tip 27b1faa0: after building the required workspace packages, the new header test plus the affected popover/context-menu tests pass 29/29.

Verdict: APPROVE
Reasoning: The prior approval still holds, the overclaiming comment/test name are corrected, and the previously unexecuted caret regression now passes on the exact restacked tip.

— Magi

[jrusso1020]

Re-review at 27b1faa046. My one finding is FIXED, and fixed the honest way.

The test is now it("keeps the caret mono at 13px, not back down at the 11px the row inherits") with a comment saying outright that it reads only this component's className and does not claim a match against the panel carets. The source comment at :79-82 was corrected the same way — it now says the size is this row's own call, and names why there is nothing to match: the node-body caret sits under a text-[9px] parent and hf-fx-preset-run-caret carries no size rule at all. That is exactly the state of those two files, so the comment is now checkable instead of aspirational. Renaming rather than reaching across components was the right pick for a test whose job is to stop a silent revert.

The CI gap I flagged is unchanged, and it is the only thing left here. At this head:

• 13 check-runs (vs 45 on #3413, which targets main). No Test, no Build, no Typecheck.
• regression = success while regression-shards = skipped, and Perf: ${{ matrix.shard }} = skipped. The green is vacuous — it is a gate passing over shards that never ran.

So the three revert-protection tests in TimelineGroupHeader.test.tsx still have not executed once, on the PR whose stated purpose is to stop this fix from being reverted again.

Correcting my own framing from the last round, because I told Vance in Slack that #3415 "is the one that can land" and that is looser than the facts: this PR's base is 08-22-fix_studio_put_the_timeline_s_portaled_surfaces_on_the_tier_the_other_portals_use, i.e. #3414's branch — not main. Merging it moves it into its parent branch, so it cannot reach main ahead of #3413 and #3414, and APPROVED/CLEAN here is scoped to that parent, not to trunk. The practical ask is narrower than "don't merge": before the stack's last rung merges into main, make sure one full-matrix run has actually covered these files — which happens naturally if each rung is re-targeted to main as its parent lands, and does not happen if the stack collapses without any rung sitting on main with these files in it.

Verdict: COMMENT
Reasoning: The finding is resolved and the comment is now accurate; the untested-tests concern is structural CI behaviour rather than anything wrong in this diff, and my earlier "can land" phrasing overstated the risk.

— Rames Jusso

[vanceingalls]

Restack note, not a code change: #3413 merged as 277fe0fa while this was in flight, so both branches were rebased onto the post-merge main. Rames' comment-interval nit from #3413 (y = 8..40 → y = 8..168 in TimelineFxPopover.test.tsx) missed that merge, so it now rides here as 836922a1 — the merged PR's assertions were already correct, only the comment interval was wrong.

Heads: #3414 836922a1, #3415 3dcb44cf (restacked onto #3414's current tip). Targeted studio tests pass at both.

The base branch was changed.