tsc (noUncheckedIndexedAccess) types marked[i]/orphaned[i] as
Element | undefined; vitest passed but bun run build failed. Narrow
before the isStylable predicate and regenerate the render-inline IIFE.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(registry,skills): surface code-highlight 0-based indexing and opacity-reveal sweep guidance
From the 2026-07-14 CLI feedback digest (skills-owner action): a user
building code teaching videos hit two authoring gaps.
1. code-highlight's `line` is intentionally zero-based (`line: 1` =
second displayed line) but the warning lived only in pr-to-video's
code-vocabulary reference — nowhere an author actually touches the
value. Call it out at the block-use sites: the `__BLOCK` declaration
itself, the registry-item description, and the motion-graphics
catalog map.
2. Opacity-only code-typing tripped `sweep_static` for that user, who
worked around it with a slow host y-drift. The sweep fingerprint
does include per-element opacity, so document the actual trap (a
reveal that settles before the sampled window, then holds a static
frame) and the idiomatic fixes (spread the reveal / keep a blinking
caret alive) in the check reference — and pin the fingerprint's
opacity sensitivity with a regression test covering both the
visibility-floor crossing and a mid-fade value change.
* docs(catalog): regenerate code-highlight page from updated registry-item description
Only the code-highlight page is committed: a full generate-catalog-pages
run also surfaces ~34 blocks missing from the git-tracked catalog index
(pre-existing drift on main), which belongs in its own chore PR.
* fix(cli): invalidate the skills nudge cache after a successful install/update/check
The passive "N skills out of date or missing" nudge reads a 24h config
cache that only the background check (on non-skills commands) ever wrote.
The skills commands themselves are excluded from the nudge pipeline, so a
successful `skills update`/install/check never refreshed or dropped the
cached verdict — the pre-install count kept printing on every other
command for up to 24h.
Reconcile commands now drop the cached verdict (counts + timestamp) so
the next command's background check re-runs for real. The offline
presence-only path deliberately keeps the cache: that run learned nothing
about freshness.
* fix(skills): win32-safe npx spawns in media-use + accurate whisper wording
The Whisper transcribe fallback and the Kokoro local-TTS delegation both
spawned a bare "npx" via execFileSync — on Windows npx is npx.cmd, which
spawn cannot exec, so both paths died with `spawnSync npx ENOENT`. Route
them through the skill's existing resolveSpawnCommand (node + npx-cli.js
on win32, no shell:true), same as the audio engine's TTS spawns.
Also corrects the "bundled with the hyperframes CLI" claim about
whisper.cpp: it is resolved from PATH / installed via Homebrew / built
from source with git+cmake on first use, and models download from
HuggingFace — nothing whisper is shipped in the package.
* feat(skills): canonical fully-silent marker + auth status exit-code docs
product-launch's Step 3.1 gate said "or the project is marked silent"
but nothing defined how to mark one, and audio.mjs unconditionally
retrieved BGM. Define the canonical marker — `music: none` in the
storyboard's top YAML block, plus no SCRIPT.md — and honor it:
audio generate produces nothing (removing stale audio_meta.json, since
absence is what assemble treats as silent), and `music: none` with
narration keeps TTS while turning BGM off.
Also documents the `auth status` exit-code contract (exit 1 while
signed out is the normal offline state, not a failure) in the
product-launch Step 0 note and the CLI skill's cloud reference.
* fix(skills): transient-init retry for standalone animation-map and contrast-report
The standalone helpers called initializeSession exactly once, so a valid
modular project — whose sub-composition timelines register asynchronously
— could hit the readiness deadline and die with the transient
"zero duration / Runtime ready: false" diagnostic the render pipeline
retries (probeStage). Add initializeSessionWithRetry to the shared
package-loader (both byte-identical copies): close the crashed session
and retry once with a fresh browser, gated by the engine's canonical
isTransientBrowserError — now re-exported from @hyperframes/producer,
with a frozen fallback pattern list for older published packages. The
"Runtime ready: true" fast-fail (a genuine authoring bug) still fails
without a retry.
* feat(skills): extend the fully-silent marker to faceless-explainer and pr-to-video
Both workflows reuse product-launch's audio model — their Step 3.1 gates
carried the same undefined "marked silent" phrase, and their (intentionally
identical) audio.mjs copies had the same unconditional BGM retrieve. Port
the `music: none` marker handling into both copies, define the marker in
their SKILL.md Step 3.1 and story-design references, and turn the
copies' "intentionally identical" header claim into a byte-identity pin
test so the next fix can't silently miss one of them.
* test(cli): reset the prune mock explicitly instead of relying on restoreAllMocks
The converge test's toHaveBeenCalledTimes(1) held only because vitest 3's
vi.restoreAllMocks() clears vi.fn() call state; vitest 4 restores spies
only, so the count would accumulate across tests and fail. Reset
pruneOrphanedLockEntries in beforeEach like the other manifest mocks —
passes under both vitest 3.2.4 (pinned) and vitest 4.
* test(skills): close review findings — package-loader pin, whisper win32 parity, quoted-none
Review follow-ups on #2476:
- package-loader.mjs byte-identity pin (the elevated concern): the two
copies now carry initializeSessionWithRetry + FALLBACK_TRANSIENT_PATTERNS,
exactly the shared-logic shape a future fix could land in one copy and
miss in the other — same enforcement as the audio.mjs pin.
- whisper win32 call-site parity: runWhisper's npx resolution lifted into
lib/npx-sync.mjs (resolveNpxInvocation, injectable params matching the
localTtsGenerate idiom) with the same three-branch coverage as the
Kokoro site — plus the hard-fail contract (throws actionably, since the
whisper fallback has no next provider to fall through to).
- quoted music: "none" pin: the vendored storyboard parser strips matching
quotes at parse time (stripQuotes), so the silent marker already accepts
the quoted spelling — pinned so that stays true.
* feat(studio,cli): per-frame board comments, self-refreshing storyboard, status-aware preview landing
Per-frame comment boxes on the storyboard board batch into
.hyperframes/frame-comments.json (a resubmit wins per frame; unconsumed
comments on other frames are kept). Submitted-but-unconsumed comments
stay visible — a toolbar banner plus a per-tile echo — until the agent
consumes the file; the banner also says what to do next (reply anything
in the agent chat).
The board keeps itself current: GET /projects/:id/signature exposes the
watcher-cached project signature, the storyboard payload carries the
signature it was derived from, and the view polls at 2s (hidden tabs
skipped, re-checked on visibility), refetching in place with no loading
flash. Posters bake the signature into their URL so tiles fill in as
sketches land and a poster that failed mid-write retries on the next
version; the empty state upgrades itself when STORYBOARD.md appears,
and its handoff prompt now points the agent at the review loop and uses
the parser's real status vocabulary (outline, not planned).
preview lands the browser on the storyboard view while the board is the
review surface — any frame built, or pure planning (srcs declared, none
on disk yet) — and on the timeline once the video is assembled.
* feat(skills): the review loop — plan, sketch, build as one shared process
hyperframes-core/references/review-loop.md is the single source for the
three-pass collaborative review: the plan proposed on a live board
(§ 1), wireframe sketches marked built with one layout question (§ 2 —
real words on plain blocks, run no CLI; a confirmed board is itself a
valid deliverable when the user asked for a storyboard, not a video),
the build dressing confirmed layouts (§ 3, worker or inline), and the
final look (§ 4). Autonomous runs skip every gate and keep one question
before render.
The three narrative workflows' Steps 3/4/6 collapse to references plus
their sketch stand-ins (captured-asset blocks for product-launch-video,
plain code panels for pr-to-video); the confirmed-sketch handoff stays
in each frame-worker prompt. general-video plans on a board for
multi-scene narrative pieces in collaborative mode — its sketch pass is
layout-before-animation with the user watching. The router treats
"I want a storyboard" as a process request rather than a route, and
closes exploratory intake by recommending a route plus how the run will
review.
The supporting contracts land next door: the comments channel (silent
submit, one reply picks it up, check the file before the words) in
brief-contract § 1; the sidecar schema and the built status rung in
storyboard-format; the mode question asked first and alone in the three
workflows' Step 0.
* feat(media-use): user memory — remembered preferences and frozen recipes
Two tiers of memory on media-use's existing two-tier storage split.
Preferences (lightweight): confirmed brief answers — destination, aspect,
language, mode, voice, style preset — recorded to the project's
.media/preferences.json (committed, the team inherits it) and promoted
to the personal ~/.media/preferences.json once the same value is
confirmed in two different projects (a sightings ledger accumulates the
cross-project evidence user-side, since project files can't see each
other). prefs.mjs get/record; merge reads project-over-user; a changed
value restarts its provenance.
Recipes (heavyweight): one approved run frozen as a named, versioned
bundle — frame.md, the storyboard skeleton (structure kept: durations,
transitions, srcs, Video direction; statuses reset to outline; content
blanked to per-frame fill-ins naming the beat's role), and the confirmed
brief values. Named folders, not content hashes: re-freezing bumps
version and archives <name>@v<N>; a freeze is already confirmed, so it
promotes to the user tier immediately. recipe.mjs freeze/list/use, plus
resolve --type recipe --entity <name> delegating like grade/lut.
16 new node --test cases; the media-use lib suite is 168/168.
* feat(skills): wire user memory into the brief and the review loop
brief-contract § 2 gains Remembered defaults: read the merged
preferences before Round 2 and let a remembered value become the
recommended option with a receipt naming its source project. Memory
changes the default, never the question — every ask-marked field still
gets asked, and what the request says this time beats what was picked
last time. Record only what the user actually confirmed (a defaulted
voice nobody chose is not an answer; a "go" that accepts the
recommended defaults is). The first record announces itself once;
after that the receipts carry the reminder. In autonomous mode a
remembered value becomes the decided value, receipt included.
The three narrative workflows read the remembered defaults before
Round 2, record the confirmed answers at the Step 0 gate, record the
chosen preset at the Step 2 gate (pr-to-video excepted — its preset is
fixed), and fall back to the remembered voice when the request names
none. general-video's discovery reads the same defaults.
Recipes wire in at both ends: Step 0 checks for a matching recipe
before the mode question — one question, plural-aware, and adopting
one fills the brief, skips the design step, and drafts the storyboard
from the frozen skeleton while every review gate still runs. The
review loop's final look (§ 4) offers the freeze once after approval,
and the confirmation teaches the recall phrase — the name is something
the system reminds the user of, never something they must remember.
The router recognizes a named recipe or "like last time" as a route.
* docs(skills): the sketch pass names check, not the deprecated validate
* feat(skills): intent-layer references — process, route briefs, capability menu, BRIEF.md format
* feat(media-use): brief skeleton as the recipe's fourth artifact; flow/storyboard preference keys
* feat(skills): the intent layer conducts every brief — workflows execute BRIEF.md
* feat(skills): retire the mode preference key; sync catalog surfaces for intent layer
* refactor(skills): dedupe router vs intent-layer guidance — one owner per rule
* feat(skills): the design ask — own spec, pick by eye from showcases, or defer
* docs(skills): the design ask says the honest line on capture routes
* feat(skills): product-launch-video absorbs website-to-video as the tour angle
* refactor(skills): keep product-launch-video pristine — a tour is brief intent, not a pipeline branch
* feat(skills): production loop + genre lenses; general-video goes freeform (route yours, laws hold)
* refactor(skills): /hyperframes is the front door - route tables and scope lists leave the workflows
* docs(skills): review-loop pass across skill catalog
* fix(cli): pass project dir to openStudioBrowser in background-server path
* feat(skills): add pitch-round reference - verbalized sampling concept gate
* feat(skills): wire pitch round into intent layer - completeness triage + route eligibility
* feat(skills): editorial capability recommendations, handoff disciplines, menu-probe split
* feat(skills): pitches carry their machinery; source-only-formed requests pitch the telling
* feat(skills): companion goes director - ceiling treatment plus blueprint/rule citation discipline
* fix(scripts): sandbox npx-leak guard - private npm global prefix keeps npx on the branch CLI
* chore(skills): resync manifest hash after formatter pass reflowed general-video tables
* fix(skills): recipe freeze reads workflow from BRIEF.md; style_preset records require workflow scope
Two holes found by a live companion-run freeze: the agent-supplied --workflow
contradicted the run's actual workflow (recipe.json said faceless-explainer,
brief-skeleton said general-video), and the style_preset lookup missed because
the preference had been recorded under the bare key.
- freezeRecipe resolves the workflow from BRIEF.md frontmatter; the flag is a
fallback for briefless projects and a contradicting flag is ignored (noted).
- recordPreference refuses a bare style_preset — the scoped key is the only
writable shape; freeze tolerates legacy bare records via read fallback.
- review-loop § 4 / media-use SKILL / brief-format wording follow the machinery.
`hyperframes render` picks up `PRODUCER_HEADLESS_SHELL_PATH` (engine
per-worker launches read it directly, and `render.ts` even propagates
the CLI-resolved executable path into it as a courtesy). But
`hyperframes check` / `snapshot` / `compare` / `grade-compare` all
route through `openSettledCompositionPage` → `ensureBrowser` →
`findFromEnv`, and `findFromEnv` only knew the CLI-native name
`HYPERFRAMES_BROWSER_PATH`.
Field report — #hyperframes-cli-feedback ts=1784095034 (win32/x64,
CLI 0.7.58): the cached `chrome-headless-shell 152.0.7928.2` crashed
with `Failed to launch the browser process: Code: 3221225595`
(`STATUS_STACK_BUFFER_OVERRUN`). Setting `PRODUCER_HEADLESS_SHELL_PATH`
to system Chrome unblocked `render`, but `check` still crashed on the
broken cached shell because it never read that env var.
Docs and deployment manifests (`skills/hyperframes-animation/adapters/
typegpu.md`, `packages/gcp-cloud-run/Dockerfile`, `examples/k8s-jobs/
Dockerfile.example`) all instruct users to set
`PRODUCER_HEADLESS_SHELL_PATH`, so the escape hatch is
documentation-blessed but was silently half-implemented on the CLI
side.
Alias it in `findFromEnv`. Tiebreak matches `render.ts:1479` —
`HYPERFRAMES_BROWSER_PATH` wins when both are set.
This is the CLI side of the symmetry #2459 is closing on the engine
(engine gaining `HYPERFRAMES_BROWSER_PATH` honoring); the two make the
alias coherent both directions.
- Sibling to #2443 (surfaces `HYPERFRAMES_BROWSER_PATH` on download
failures).
- Not the same class as #2040 (arm64 pin), #2078 (SIGTRAP), or #2082
(launch crash rewrap) — those are download / launch fixes; this is
the env-var alias gap.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deepwork's request-changes on #2411 (twice): deFallbackReason's blank/psnr
split still ran /blank/i.test(err.message) even after this PR's stated goal
of moving off message-text parsing — a reworded message, a translated
string, or a differently-shaped error crossing a module boundary could
silently relabel a blank failure as psnr (or vice versa), corrupting the
soak's telemetry taxonomy.
DrawElementVerificationDetails now carries a required `kind: "blank" | "psnr"`
field, set at all three real throw sites in captureStreamingStage.ts. The
orchestrator derives deFallbackReason from getDrawElementVerificationDetails's
kind instead of regexing the message. Making `kind` a required constructor
argument means any future throw site that omits it fails to compile, closing
the gap for good rather than just at today's three call sites.
New tests in frameCapture.test.ts prove message-independence directly: kind
survives a reworded message that says neither "blank" nor "psnr", and stays
correctly "psnr" even when the message adversarially contains the substring
"blank" — the exact scenario a regex-based classifier would get wrong.
Review feedback on #2411 (Rames): the crash-survival RenderCaptureObservability
mirror passed deFallbackFailedDb raw/unrounded while the render_complete
perfSummary path rounded to 1 decimal — the same underlying PSNR could ship two
different values to PostHog depending on which event fired. Extracted the
existing inline round/clamp expression (previously duplicated for verifyMinDb
and fallbackFailedDb) into a shared roundDb helper, applied once at the single
point deFallbackFailedDb is derived from the thrown error so both downstream
consumers agree.
Also threads verifyThresholdDb (captured on the error but never propagated,
per the nit) through DrawElementPerfInput/RenderCaptureObservability/render.ts/
telemetry as de_fallback_threshold_db on both events — the HF_DE_VERIFY_MIN_DB
value the failing dB breached, letting ops read "28.4dB failed a 32dB
threshold" directly instead of cross-referencing config.
de_fallback_reason only told you the fallback happened (blank/psnr/oom/
capture_error), not the failing PSNR or frame index — that data existed as
text inside the thrown error's message and was discarded on the way to
telemetry. DrawElementVerificationError now carries structured
frameIndex/failedDb/verifyThresholdDb; the orchestrator reads them via the
new getDrawElementVerificationDetails helper instead of regexing message
text, and both telemetry surfaces (the render_complete perfSummary path and
the crash-survival RenderCaptureObservability mirror) emit
de_fallback_failed_db / de_fallback_frame_index.
Needed to distinguish "32dB vs the 32dB threshold, tune it" from "12dB real
corruption, investigate" during the parallel-router soak — currently that
distinction is invisible.
## What
- cap static-dedup verification at 15 seconds of wall-clock time
- disable the optimization and continue normal capture when the verification budget is exhausted
- add a regression that models the reported 350-second / ~8,400-frame alpha render
## Why
Static-frame verification uses full-page screenshots. Its existing screenshot-count budget still scales with composition duration, so a long composition can spend minutes proving an optimization before frame capture starts. The reported 350.35-second ProRes alpha render spent about eight minutes in this phase before safely disabling dedup.
## How
The verifier now records a deadline before seeking verification frames. It checks the deadline before every full-page capture and returns the existing fail-closed `budgetExhausted` result when time is exhausted. This keeps the existing density-based safety checks while bounding their startup cost.
## Test plan
- [x] Unit tests added/updated
- [ ] Manual testing performed
- [ ] Documentation updated (not applicable)
- [x] Focused engine test: 9/9 passed
- [x] Engine typecheck passed
- [x] Pre-commit lint, format, tracked-artifact, fallow, and typecheck gates passed
- [x] Full engine suite: 988 passed, 3 skipped; 2 pre-existing environment failures because host FFmpeg 4.2 lacks `-fps_mode`
* fix(lint): stop CSS comments in <style> from manufacturing phantom root tags
extractOpenTags scans raw source text with a flat regex that has no
concept of <style>/<script> block boundaries, so a CSS comment like
`/* <g> wrapper */` inside a <style> block reads as a real open tag.
findRootTag consumes that flat tag list and only skips tags literally
named script/style/meta/link/title, so the phantom <g> tag (not in
that skip list) wins the "first non-ignored body tag" search and gets
returned as the composition root instead of the real one that follows.
This manufactured root_missing_composition_id and root_missing_dimensions
(the phantom tag has neither) plus head_leaked_text (the leaked-text
scan slices up to the phantom tag's position, landing inside the
<style> block before its real closing tag, so the raw CSS text reads
as leaked markup) on an otherwise valid sub-composition — reported
with an exact bisected repro: a <template>-wrapped SVG sub-composition
whose <style> block comments reference an inner <g> element.
Fix: compute <style>/<script> content spans up front (reusing the
existing extractBlocks + STYLE_BLOCK_PATTERN/SCRIPT_BLOCK_PATTERN) and
skip any TAG_PATTERN match that falls inside one, before it ever
reaches findRootTag or any other extractOpenTags consumer. Same shape
as the prior fix for a leading <svg> defs block being mistaken for the
root (8ee4b7df) — this closes a sibling gap in the same function.
Test: new regression case with a <style> block containing a `/* <g> */`
comment ahead of an <svg data-composition-id> root, asserting none of
the three findings fire. Full lint package suite (318 tests) passes.
* feat(lint): flag duplicate data-composition-id values
Declaring data-composition-id on more than one element (commonly the <meta>
tag from the quickstart template AND the root <div> added to satisfy
root_missing_composition_id) is a silent collision: `compositions --json`
returns two entries for the same id (one duration:0) and inspect/snapshot
crash with "Cannot read properties of undefined (reading totalDuration)".
Lint passed clean through all of it.
New rule `duplicate_composition_id`: group elements by data-composition-id
value and error on any value shared by 2+ elements, naming the id and calling
out the meta-vs-root collision in the fixHint. 3 tests: dup fires, single id
passes, two distinct ids don't collide. (Implemented via Codex; verified
independently: 111 lint tests pass, oxfmt/oxlint clean.)
* fix(audits): avoid caption false positives
* fix(lint): ignore proxy-label tween overlaps
* fix(cli): preserve the five-percent text audit floor
* fix(lint): preserve proxy identity across lexical scopes
* fix(cli): audit only directly painted text
* fix(lint): compare live composition ids canonically
* fix(lint): preserve expanded proxy identities
* fix(cli): measure directly painted text geometry
* fix(lint): preserve first duplicate attribute value
* fix(lint): keep shared proxy identity across helpers
* fix(parsers): preserve expanded proxy identity
* fix(lint): decode composition IDs consistently
* fix(fonts): subset Google Fonts to composition text
* fix(producer): skip slow TTC recompression
* fix(fonts): include decoded composition text in subsets
Miga review nit on #2443. Existing test only covered darwin/arm64 by
default; parameterize via it.each across process.platform so each platform's
Chrome-path hint from browserPathHintForPlatform is asserted in the rethrown
error message.
— Via
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
CI's file-size check (which diffs against origin/main, not per-commit like
the local lefthook gate) flagged useDomEditCommits.ts at 602 lines. Extracted
the standalone atomic-patch-batch helpers (formatUnsafeFieldList,
getErrorDetail, readErrorResponseBody, formatPatchRejectionMessage,
patchElementBatches, batchesAreInlineStyleOnly,
AtomicElementPatchConvergenceError) into useDomEditCommitsHelpers.ts — none
of them close over hook state, so this is a pure move. useDomEditCommits.ts
is now 451 lines.
Typecheck/oxlint/oxfmt clean; useDomEditCommits.test.tsx (28 tests) and the
full studio suite unaffected.
Fixes real bugs from two independent re-reviews (#2225 @ 65954c3804,
#2416 @ beaf4ffbf6):
- FlatTimingRow's pinRange committed a pinned start+duration range through
TWO sequential onSetAttribute calls. Each resolves domEditSelection fresh
from current hook state, so a selection change between the two awaits
could misdirect the second write at the newly-selected element instead of
the one being edited, and a failure of just the second call left the pair
half-applied (inconsistent inferred/explicit state). Added
commitDataAttributes/handleDomAttributesCommit (mirroring
onCommitAnimatedProperties's same-shaped fix for GSAP property batches):
one PatchOperation[] persist call against an explicit, caller-supplied
selection — not the "current" one — threaded through as the new optional
onSetAttributes prop. pinRange uses it when provided, falls back to the
old sequential behavior otherwise.
- Hide All silently dropped nested sub-composition children: a selection
inside a sub-comp with no timeline-store entry of its own resolves to a
virtual `sourceFile#domId` key (the fallback branch exists so the
expansion hook can later resolve it via clipParentMap), but
toggleTimelineElementHidden only searched the RAW store list, which never
contains that key. useTimelineElementVisibilityEditing now resolves
against useExpandedTimelineElements() instead, matching the track-based
toggle's existing approach — the expanded list synthesizes a real,
patchable TimelineElement (matching key/domId/sourceFile) for each visible
child whenever its host is currently expanded.
- Two composition hosts importing the same sub-composition collapsed to
the first one: findMatchingTimelineElementId ORed domId/selector/
compositionSrc matches with equal priority in a single per-element scan,
so `.find()` could stop at an EARLIER, unrelated host that merely shared
the compositionSrc, before the scan ever reached the correct domId/
selector match further down the list. Restructured to try domId, then
selector, across the WHOLE list first; compositionSrc-only matching is
now a true last resort for when neither identifies a specific element.
- FlatSlider's native pointercancel handler (a platform-level gesture abort
— scroll/touch takeover, pen leaving range) manually duplicated the
pointer-capture release logic instead of calling cancelDrag, so it never
reverted to the pre-drag value — leaving whatever intermediate position
the pointer last reached committed, unlike the Escape/right-click paths
added in the previous round. Now calls cancelDrag directly.
- useColorGradingController's flushPendingPersist read identityKeyRef.current
fresh at flush time rather than a value snapshotted when the edit was
scheduled. Defensive fix: added pendingPersistIdentityRef, set alongside
pendingPersistValueRef in commitColorGrading, read by flushPendingPersist
instead of the live ref — closes the gap regardless of how unlikely the
actual race is given the identity-cleanup effect's existing eager-flush
behavior.
Two prior findings re-verified as already fixed further up this same
Graphite stack (not re-fixed here, per established stack-order handling):
metadata-cache negative-caching (267cdfce1) and cross-file
selectionIdentityKey (6f40e03a1), both landing after #2225's reviewed head.
StudioRightPanel.tsx crossed the 600-line file-size gate after wiring the
new onSetAttributes prop through; extracted the inspector split-pane resize
handlers (previously inlined) into their own useInspectorSplitResize hook.
New regression tests: repeated-composition-host resolution, atomic vs.
fallback pinRange commit paths, pointercancel revert. Full studio suite
still at the known pre-existing 55-failure baseline, zero new regressions.
Typecheck/oxlint/oxfmt clean.
A fresh full-stack re-review checked 15 PR heads independently. Cross-
checked all 9 remaining claims against the actual current tip:
- #2120 (id/selector key qualification, Hide All no-op/race, variable
parity), #2121 (opacity-zero fallback), #2122 (GSAP preview sibling
resolution, scrub-label), #2124/#2126 (negative metadata cache), and
the keyboard-access half of #2121/#2186 were all already fixed by a
later commit in this same stack (65954c380, PR #2225) — the reviewed
heads predate it. Verified each in the current source rather than
taking the isolated-head review at face value.
- #2186's "no ESC/right-click cancel during drag" was the one claim that
held up: FlatSlider had keyboard arrow-key support but no way to abort
an in-progress pointer drag. Escape now reverts to the pre-drag value
and releases pointer capture; a right-click (contextmenu) during a drag
does the same instead of committing whatever position the pointer last
reached while the native context menu opens over the slider. Both go
through commitDraft (not just a visual reset) since the drag's leading-
edge commit in onPointerDown may already have applied an intermediate
value that needs actually undoing, not just hiding.
propertyPanelFlatPrimitives.tsx crossed the 600-line file-size gate after
this change; extracted FlatSelectRow into its own file, matching the
FlatToggle/FlatMaskInsetRows precedent from earlier in this stack.
New regression tests for Escape-cancel and contextmenu-cancel. Full
studio suite still at the known pre-existing 55-failure baseline, zero
new regressions. Typecheck/oxlint/oxfmt clean.
Fixes the four blockers from the #2416 re-review at head d6a40c38b:
- FlatSlider's onPointerUp calls releasePointerCapture() explicitly, which
fires lostpointercapture SYNCHRONOUSLY in real browsers — the prior
unconditional onLostPointerCapture resync ran mid-onPointerUp, flipping
draggingRef false before onPointerUp's own check, silently dropping every
normal drag-release's final commitDraft(). happy-dom doesn't replicate the
synchronous cascade, so this shipped without a failing test. Added an
explicitReleaseRef flag set right before each deliberate
releasePointerCapture() call so onLostPointerCapture can tell "our own
release, caller's logic already handles it" apart from a genuine external
capture loss. Added a regression test that monkey-patches
releasePointerCapture to reproduce the real-browser ordering.
- persistColorGradingValue read onSetAttributeLiveRef.current (reassigned
every render) instead of the callback live when the debounced edit was
scheduled — a timer for element A firing after a re-render for element B
would wrongly call B's callback with A's data. Removed the ref; the
callback is now an explicit parameter captured by commitColorGrading's own
closure (added to its useCallback deps) and threaded through to
persistColorGradingValue and flushPendingPersist.
- flushPendingPersist passed () => true as its isLatestAttempt checker,
bypassing the per-commit version guard entirely. Now calls
bumpDomEditCommitVersion(gradingVersionRef) like a regular debounced
commit, so a newer edit landing before the flushed write settles still
wins the race.
- The selection-identity cleanup effect stopped clearing statusTimersRef
during an earlier refactor — stale RUNTIME_STATUS_REFRESH_DELAYS timers
for an outgoing element could fire after switching selection and stamp
the new element's runtimeStatus with the old element's answer. Restored
the clear in the same effect cleanup.
Also gave the Custom LUT and "Copy grade to" scope <select> controls
aria-labels — both had their visible text in a sibling span/text node, so
neither had an accessible name.
Full studio suite still at the known pre-existing 55-failure baseline
(variablePromoteIntegration, useGsapPropertyDebounce, sdkCutover(Parity),
sdkResolverShadow), zero new regressions. Typecheck, oxlint, oxfmt clean.
Fixes three of the adversarial findings from the third #2416 tip
re-review:
- Grade rollback was identity-scoped but not attempt-scoped: two edits on
the SAME element (e.g. drag Exposure, then Contrast, before Exposure's
persist settles) could have the earlier edit's late completion stamp
confirmedGradingRef with its now-superseded value, or revert `grading`
out from under the newer optimistic edit. Added a monotonic per-commit
version via the existing bumpDomEditCommitVersion primitive (the same
one the DOM-attribute commit runner uses for the identical race) —
persistColorGradingValue now checks both identity AND "is this still the
latest attempt for this element" before applying any effect.
- The render-phase identity-reset block consumed shared mutable state
(clearing the pending-persist timer, reading and nulling
pendingPersistValueRef) directly during render. Adjusting STATE during
render this way is React's documented pattern and safe to repeat, but
consuming a ref this way is not: if React discarded/interrupted that
specific render before it committed, the timer would already be
cancelled and the pending value already nulled, with no corresponding
effect ever running to compensate, silently losing the edit. Replaced
with the idiomatic pattern for "clean up a per-identity resource when it
changes" — a useEffect keyed on identityKey whose CLEANUP performs the
cancellation/flush. A cleanup only ever runs for the effect instance
that actually committed, closing the gap entirely. The render-phase
block now only performs pure, idempotent state resets.
- FlatSelectRow's Preset row passes label="" (the visible "Preset" text is
a sibling span, to avoid rendering it twice) which left the underlying
<select> with no accessible name at all. Added a dedicated `ariaLabel`
prop, distinct from the visible `label`, so a caller can supply a name
without a duplicate visible label.
Also hardened FlatSlider's lostpointercapture handling: it now resyncs
the draft directly from a latestValueRef immediately, instead of only
clearing the dragging flag and waiting for the separate [value]-keyed
effect to notice — closing a narrow ordering gap where a value change
arriving while still dragging, followed by capture loss with no further
render, could otherwise leave the knob stuck.
propertyPanelFlatPrimitives.tsx crossed the 600-line file-size gate after
these changes; extracted FlatToggle (and its tests) into their own files,
matching the FlatMaskInsetRows precedent from an earlier commit in this
stack.
New/updated regression tests: same-element version race, Preset select's
aria-label. Full studio suite still at the known pre-existing 55-failure
baseline, zero regressions.
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).
