Fixes two of the three adversarial findings from the second #2416 tip
re-review; the third is a pre-existing runtime-protocol gap, explained in
the PR thread rather than patched here.
- The Grade rollback added in the previous commit could never fire through
the real Studio callback: runDomEditCommit (the shared commit runner used
by every data-attribute commit, not just Grade) catches persist failures
internally and always resolves, reporting outcome only via its own
onError side effect. A caller awaiting the promise never sees a
rejection, so the revert-on-reject logic was dead code against the
actual app. Added an optional onSettled(ok) callback to
DomEditCommitRunnerConfig (purely additive — every existing caller that
doesn't pass it is unaffected) and threaded it through
commitDataAttribute -> handleDomAttributeLiveCommit -> the
onSetAttributeLive prop type (now accepts an optional 3rd argument) ->
useColorGradingController, which now drives the revert from the real
signal. The promise-rejection path stays as a fallback for any other
implementation of onSetAttributeLive that rejects instead.
- Selection flushing performed a real side effect (writing the outgoing
element's pending edit) during the render-phase identity-reset block.
Adjusting STATE during render (comparing against a ref) is React's
documented pattern, but it doesn't license actual I/O — React can invoke
render more than once per commit, which could double-fire or misorder
the write. The reset block now only enqueues the flush (a pure ref
write); a new effect keyed on the identity performs it after commit.
- Async persist completions (both the onSettled callback and its promise-
rejection fallback) now capture the identity key the attempt was made
for and check it against the CURRENT identity before touching
confirmedGradingRef/grading/runtimeStatus. Without this, a persist that
settles after selection has moved on to a THIRD element could clobber
that element's freshly-reset state with a result that belongs to an
element no longer selected.
Not fixed here: the runtime Grade target (HfColorGradingTarget, used by
core's resolveTarget to find the DOM element inside the preview iframe)
has no source-file/composition-scope discriminator, matching the same gap
selectionIdentityKey had before this stack — but fixing it means changing
a wire-protocol type shared across core/player/studio and the legacy
ColorGradingSection too. hfId (checked first, before id/selector) is
minted uniquely per element at parse time in the common case, so this is
a narrow residual risk for hfId-less same-selector elements across
different source files, not a regression introduced by this stack.
Flagged as a follow-up in the PR thread.
New/updated regression tests: real onSettled(false) path (distinct from
the promise-rejection fallback), and a stale in-flight persist settling
after selection has moved on twice more. Full studio suite still at the
known pre-existing 55-failure baseline, zero regressions.
Fixes the Deepwork tip re-review's four remaining blockers plus its
additive findings:
- selectionIdentityKey: add sourceFile as a 5th identity component. The
same local id/selector can legitimately recur across different
composition files (host vs. an inlined sub-composition, or two unrelated
sub-comps) — without sourceFile, those collided onto the same identity
key and reused stale controller state across a selection change that
should have reset it.
- useColorGradingController: flush (not discard) a pending Grade edit when
selection changes before the 350ms debounce fires. The prior fix
correctly stopped it from landing on the WRONG (new) target, but
cancelling outright silently dropped the user's in-flight edit instead of
writing it to the element it was authored for — using the
onSetAttributeLive closure captured for the outgoing render, which
(via commitDataAttribute's own useCallback deps) is still bound to the
outgoing selection.
- useColorGradingController: revert to the last confirmed-good grading when
a persist rejects, instead of leaving the optimistic (never-actually-
saved) value showing indefinitely. Tracks a separate
confirmedGradingRef, updated only on a successful persist.
- FlatSelectRow: disable the reset button when the row itself is disabled
(it previously ignored disabled entirely, same class of bug as the
FlatSlider reset button fixed earlier) and give the underlying <select>
an aria-label from the row's label text.
- FlatSlider: handle lostpointercapture the same as pointercancel — capture
can be lost without either firing first (another element steals it, or
the browser reclaims it for a scroll/touch gesture), which previously
left the dragging flag stuck and the knob permanently unable to sync to
external value changes.
New regression tests for all of the above; full studio suite still at the
known pre-existing baseline (55 failures unrelated to this stack).
Fixes issues raised in the Deepwork re-review of #2120-#2190 that weren't
covered by #2225's earlier fix pass:
- useColorGradingController: reset grading/compare/mediaMetadata state (and
cancel pending persist/status timers) when selection changes to a
different element — this hook is called unconditionally on every render
(unlike legacy ColorGradingSection, remounted via a selectionIdentityKey
React key), so switching selection reused the previous element's state.
- useColorGradingController: stop permanently caching a non-OK
/media/metadata response as null — a transient server error poisoned the
HDR banner for that asset for the whole page lifetime.
- FlatSelectRow: preserve a valid authored value outside the preset list
(e.g. mix-blend-mode: difference, an arbitrary object-position) instead of
silently misrepresenting it as the first preset — touching the control
would overwrite real persisted state.
- FlatSlider: the throttled trailing commit now reads onCommit through a
ref updated every render instead of closing over it at schedule time — a
caller whose onCommit spreads other current state (Grade's per-detail
commits) could otherwise have a delayed commit revert whatever the user
changed on a different control in the same 40ms window.
- FlatSlider: flush a still-queued trailing commit on unmount instead of
dropping it, and disable the reset button when the slider itself is
disabled.
- FlatSlider: add touch-action: none to the track so touch drags don't
compete with page scroll.
- FlatColorGradingAccessory: clean up the compare-hold's window listeners
on unmount, not only on release — switching selection mid-hold used to
leak them.
- Align (flat Text): re-clicking the option already visually active for a
logical start/end value no longer rewrites it to the physical left/right,
preserving RTL semantics.
- FlatSegmentedRow: give every option an accessible name and aria-pressed
state — two visually-identical glyph buttons (upright/italic "A") had no
way to be told apart by assistive tech.
- PropertyPanelFlat: the panel body falls back to its own scroll when the
collapsed group headers alone exceed the available height, so groups
can't become permanently unreachable in a short pane.
New regression tests for all of the above; full studio suite at the known
pre-existing baseline (55 failures unrelated to this stack).
A pure debounce resets its timer on every pointermove, so a real drag
(events faster than 40ms apart) never commits until the pointer pauses
or lifts — killing live preview updates mid-drag. Throttle with a
leading-edge commit + trailing flush instead.
Only onPointerDown was wired, so dragging the knob/track only ever
committed the initial click position — nothing tracked the pointer
after that. Uses the Pointer Capture API (setPointerCapture on
pointerdown, onPointerMove while captured, release on pointerup) so
the value follows the cursor continuously during a drag, matching how
the legacy native <input type="range"> control behaves for free.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The track's visible line was only 2px tall, and pointerdown was bound
directly to that thin element, making it hard to grab. The hit area is
now 20px tall (a wrapping div) with the visible line rendered as a
thin decorative child, centered inside it — the ratio math only reads
left/width so click accuracy is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a fast (120ms) CSS entrance animation for flat inspector accordion
group headers/body, gated to the group actually toggling (not derived
from remounting alone) to avoid a Chromium reflow quirk that otherwise
replays the animation on untouched collapsed siblings.
Collapsed group headers render in fixed, non-scrolling document flow
above and below the open group; only the open group's own content
scrolls, in a dedicated region. Also fixes the flat inspector footer's
missing background.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The flat inspector rendered its new Style/Grade groups AND the legacy
ColorGradingSection/StyleSections components a second time below them,
visibly doubling every control. Remove the now-redundant legacy render
call sites (and their now-unused imports) from PropertyPanelFlat.tsx;
those components stay intact for the legacy (flag-off) PropertyPanel.
FlatTextSection's multi-field branch (textFields.length > 1) now renders
FlatTextLayerList (Task 5) + the existing single-field FlatTextFieldEditor
for the active field, tracked via new local activeFieldKey state that
resyncs (useEffect) when the active field disappears from props. This
retires the legacy TextSection delegation entirely for that case; the
TextSection import is removed from propertyPanelFlatTextSection.tsx since
nothing else in the file referenced it.
Also updates propertyPanelSections.test.tsx and PropertyPanel.test.tsx,
which exercised/documented the old multi-field-falls-back-to-legacy-
TextSection behavior in comments and test titles — reworded to describe
the new flat path (assertions were already compatible and still pass).
Flag for reviewer: hideOwnHeading on the legacy TextSection component
(propertyPanelSections.tsx) was added in an earlier plan specifically for
this now-removed call site. It has no remaining consumer after this task
lands (PropertyPanel.tsx's legacy caller doesn't pass it). Left in place
per brief instruction — not deleting unilaterally, since that's a scope
decision for whoever reviews this task.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review proved the existing test didn't catch a broken stopPropagation
by temporarily removing it and confirming the suite still passed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review of the pin-aware group list refactor flagged the test name
claiming the group "closes" on unpin — it doesn't assert that, and
structurally the group re-opens (togglePin never touches openGroupId).
Retitled to describe only the return-to-stack behavior actually tested.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reads/writes the per-element-kind pinned-groups map added to
studioUiPreferences in the prior task, read-modify-writing the whole
map since writeStudioUiPreferences only shallow-merges top-level keys.
Also adds a not-busy click assertion for the Apply button so
onApplyToScope is exercised end-to-end, not just its disabled state.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Also hoists the pointerdown-drag/reset-click test scaffold shared by the
new Roundness tests and the existing Contrast/Exposure tests into helpers
(findRowByText/dragSliderTrack/clickSliderReset), and exempts pre-existing,
branch-inherited fallow findings unrelated to this task (TextFieldEditor
complexity from earlier Text-inspector commits; test-scaffold duplication
across four flat-inspector-series test files from Plans 2-4) via
.fallowrc.jsonc, per this repo's established convention for line-shift/
inherited findings.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review of Task 6 flagged the exposure/other-key scale ternary as always
resolving to 100 on both arms.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Follow-up to 684ec4e87: that fix corrected the seek target for Layout's
keyframe gutter via deriveElementTiming, but currentPct — which drives
KeyframeNavigation's diamond active/inactive state and prev/next arrow
targeting — still used PropertyPanel's naive elStart=0/elDuration=1
basis. For an element with animations but no explicit data-duration,
seeking to a keyframe's real absolute time no longer lit that
keyframe's diamond as active, and the prev/next arrows targeted the
wrong keyframes.
Thread currentTime into PropertyPanelFlat (swapping the now-redundant
currentPct prop 1-for-1, so PropertyPanel.tsx's line count is
unchanged) and recompute currentPct there from the same
deriveElementTiming basis already used for the seek fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Whole-plan coherence review (Plan 3a Layout + Plan 3b Motion) found that
Layout's keyframe gutter and Motion's Timing row independently derived an
element's start/duration and disagreed whenever an element had animations
but no explicit data-duration: Motion correctly inferred the range from the
element's GSAP tweens, while Layout's keyframe gutter fell back to a naive
`duration ?? 1`, so clicking a keyframe percentage in Layout could seek to a
different absolute time than what Motion's Timing row displayed.
Extract deriveElementTiming (propertyPanelFlatTimingDerivation.ts) as the
single shared basis both paths now consume: FlatTimingRow (Motion) and
PropertyPanelFlat's own elStart/elDuration (Layout's keyframe gutter and 3D
Transform block). PropertyPanelFlat now recomputes this basis itself from
its own element/gsapAnimations props instead of trusting the parent's naive
value, so PropertyPanel.tsx (and its legacy non-flat panel) is untouched.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Thread the Layout-group values through PropertyPanel -> PropertyPanelFlat
and add the third FlatGroup to the one-open/pin accordion (unconditional,
matching legacy Layout). Default-open Layout when neither Text nor Style
applies.
Fix the Flex double-render: the legacy StyleSections still renders its own
Flex Section, and the new flat Layout group renders its own LayoutFlexBlock.
Add an additive optional hideFlex prop to StyleSections and pass it on the
flat path so Flex renders exactly once (from the flat Layout group). Non-flat
callers omit it and are unchanged.
Extract the shared onLivePreviewProps closure into gsapLivePreview.ts (it was
duplicated inline in the legacy path) so PropertyPanel.tsx stays within the
600-LOC studio gate.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## What
First PR in a 6-PR stack migrating Studio's right-panel property inspector from an always-expanded stacked-sections layout to a "flat" one-open-at-a-time accordion. This PR lays the foundation: the `STUDIO_FLAT_INSPECTOR_ENABLED` feature flag, the accordion primitives (`FlatRow`, `FlatSegmentedRow`, `FlatGroup`, `PinnedZoneDivider`), the flat identity header/footer, and the first migrated group — Text.
Stack: #2120 (this) → #2121 (Style) → #2122 (Layout+Motion) → #2123 (Media) → #2124 (Grade) → #2125 (Pinning + multi-field Text).
## Why
The legacy inspector renders every applicable section expanded at once, which gets unwieldy as an element accumulates properties across style/layout/motion/media/grade. The flat redesign shows one section at a time (plus pinned sections), matching a design handoff mock.
## How
- `FlatGroup` owns the one-open accordion state (`openGroupId`/`onToggleOpen`) and pin affordance (`onTogglePin`), styled per the design mock.
- `FlatTextSection` is the first migrated group and the reference implementation every later group's task followed for the `isOpen`/`onToggleOpen`/`onTogglePin`/`summary` wiring pattern.
- Includes a same-PR bugfix (found via live browser testing, not caught by any automated test): the Text `FlatGroup` was rendering unconditionally regardless of element type (empty for non-text elements), and the multi-field fallback doubled the "Text" heading. Fixed by gating on `isTextEditableSelection` and adding a `hideOwnHeading` prop to the legacy `TextSection` fallback.
- Entirely gated behind `STUDIO_FLAT_INSPECTOR_ENABLED` (default off) — the legacy panel is untouched and remains the default for all users.
## Test plan
- Every primitive and the Text group have dedicated Vitest suites using real DOM events (click/pointerdown) with exact assertions, not shallow snapshots.
- Manually verified in Studio via live browser testing against the design mock (this is what caught the bugfix above).
- Full monorepo test suite green; `oxlint`/`oxfmt` clean; this repo's `fallow` complexity/duplication gate passes.
- [x] Unit tests added/updated
- [x] Manual testing performed
- [ ] Documentation updated (not applicable — internal Studio UI behind an off-by-default flag)
## What
Already-scaffolded HyperFrames projects pin an exact `hyperframes@X.Y.Z` in `package.json` scripts (`init.ts`), frozen at scaffold time — and the update-available notice is suppressed on non-TTY shells, exactly how agents invoke the CLI. So a large tail of projects sits on months-old versions, invisible and stuck, never seeing later render/router fixes.
The pin itself is deliberate (a video project should re-render identically across CLI versions), so this PR keeps it and instead gives projects a path off it:
1. **`rewriteProjectPinnedScripts` / `readPinnedHyperframesVersions`** (`packages/cli/src/utils/projectPin.ts`) — pure helpers that rewrite/read `hyperframes@<version>` pins in a `package.json` scripts object.
2. **`hyperframes upgrade --project [dir]`** — bumps a project's pinned scripts to npm-latest in one command (`--check` reports the delta without writing, `--json` for `{ changed, from, to, path }`).
3. **`printStalePinNotice`** — a throttled (once/24h), non-TTY-visible notice (unlike the existing update notice, which non-TTY shells suppress) that fires when the *current* project's pin is stale, pointing at `upgrade --project`.
4. **Skill instruction** (`skills/hyperframes-cli/SKILL.md` + reference) — tells agents to check for and bump a stale project pin via the **unpinned** `npx hyperframes@latest upgrade --project`.
5. **Scaffold templates** (`CLAUDE.md`/`AGENTS.md`) — new projects get the same guidance baked in from day one.
## Why
Only the global skill (piece 4) invoking the unpinned `npx hyperframes@latest upgrade --project` (piece 2) reaches projects that are *already* frozen on an old pin — a project pinned to an old CLI version never runs the new notice code (piece 3) or sees the new template text (piece 5). Those two are forward-only: they stop the bleed on projects scaffolded from here on, but the skill instruction is the only lever that reaches the existing backlog.
## How
`isSafeVersion` was extracted out of `updateCheck.ts` into its own `safeVersion.ts` module — `projectPin.ts` needs it and `updateCheck.ts` needs `projectPin.ts`'s `readPinnedHyperframesVersions`, so keeping `isSafeVersion` in `updateCheck.ts` created a circular import between the two files.
## Test plan
- [x] Unit tests added/updated (`projectPin.test.ts`, `upgrade.project.test.ts`, `updateCheck.stalepin.test.ts`) — TDD, all passing
- [x] Full `packages/cli` suite green (132 files / 1651 tests), `tsc --noEmit` clean, `bun run build` succeeds
- [x] Manual smoke test: `upgrade --project --check --json` reports the delta without writing; `upgrade --project` rewrites the pinned scripts in place
- [ ] Documentation updated — skill + scaffold templates updated in this PR; `CLAUDE.md`/`AGENTS.md` template parity verified with `diff -q`
Deferred (left for a separate decision, not in this PR): `npm deprecate hyperframes@"<0.7.53"` — reaches frozen projects with no skill loaded, but is a live, hard-to-reverse action against published packages that needs an explicit human call on cutoff version + message.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Review round on PR #2442:
- miguel (blocker): the cross-file eligibility rule only guarded the
dom-edit tripwire; recordResolverParity and
recordAnimationResolverParity ran before wrongCompositionFile at
every cutover surface, so cross-file ops still emitted false
element_not_found (id present in the OTHER file's source passes the
runtime-node filter) and polluted the attempt denominator. The rule
now lives in one shared isCrossFileEdit guard applied by all three
entry points, wired with { targetPath, compositionPath } at all six
sdkCutover call sites (timing, timing-batch, gsap add/set/remove,
keyframe chokepoint, delete).
- Rames (race): the disk-truth read is now dispatched SYNCHRONOUSLY in
the same prologue as the miss check, before control returns to the
caller whose cutover persist writes the same file moments later — a
post-write read would see a remove op's target legitimately gone and
misclassify it as a genuine divergence. Sync reader throws become
rejections (IIFE), not exceptions into the swallow-all catch.
- Rames (parse failure): openComposition failure inside the disk check
now fails open as sourceReadFailed (unparseable source is not ground
truth), instead of the outer catch dropping the divergence event
entirely.
recordResolverParity's source check extracted to checkHfIdInSource
(complexity gate), mirroring checkAnimationIdOnDisk's error discipline.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two resolver-shadow noise classes from production telemetry:
- Cross-file guard (0.7.41: 479 false element_not_found from ONE
session): the dom-edit tripwire ran for edits targeting a different
file than the session models. The cutover gates already decline these
(wrongCompositionFile); the tripwire now skips the same way — no
event, no attempt, since the op structurally cannot cut over.
- Stale-session disambiguation (0.7.48: 53 animation_not_found across
keyframe ops): the GSAP panel derives animationIds from the CURRENT
on-disk script every render, while the session's parsed id space
dates from the last reload. Position edits shift every
selector-method-position id, so panel ops landing before the reload
target ids the session has never seen. Parser id-space parity was
verified across legacy/acorn read/write paths (9 script shapes) —
the ids agree; the session is just behind. On a miss with a reader
wired, recordAnimationResolverParity now re-parses the on-disk file:
a hit there = stale session (suppress); a miss there = genuine
divergence, tagged diskChecked so the dashboard can trust the class.
Attempt-counter machinery moved to sdkResolverAttempts.ts (600-LOC
studio file gate); re-exported from sdkResolverShadow for API compat.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The flat inspector's Text FlatGroup rendered unconditionally, showing an
empty "Text" header for non-text elements (image, video, etc). Gate it on
isTextEditableSelection(element) so it disappears entirely when there's no
text to edit.
Also, the legacy multi-field TextSection (used as a fallback when an
element has 2+ text fields) rendered its own internal "Text" heading
nested inside the new flat Text FlatGroup, producing a doubled "Text"
heading. Add a hideOwnHeading prop to TextSection (default false, so its
other — legacy, non-flat — call site is unaffected) and pass it from
FlatTextSection's fallback path.
Vance Ingalls