feat(studio): add media treatment inspector#2755
Conversation
There was a problem hiding this comment.
Pull request overview
Adds “media treatment” tooling to the Studio flat inspector so selected media can be graded, given shader effects, and augmented with Registry overlay components—while keeping persistence routed through existing edit/registry install paths.
Changes:
- Introduces color grading preview batching + animated hover previews via the preview iframe runtime bridge.
- Adds new flat inspector sections for Grade presets, Effects (families + controls), and Registry-driven Overlays install.
- Extends block installation to support explicit overlay placement (start/duration/track) and refactors install/insert logic with targeted tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/studio/src/utils/blockInstaller.ts | Refactors Registry install + sub-comp insertion; adds placement duration/optional track support for overlays. |
| packages/studio/src/utils/blockInstaller.test.ts | Covers explicit duration/track placement behavior for Registry components. |
| packages/studio/src/hooks/useBlockHandlers.ts | Adds handleAddMediaOverlay to install overlays into the selected media’s composition/timing. |
| packages/studio/src/components/StudioRightPanel.tsx | Wires an optional onAddMediaOverlay handler through the right panel to the property panel. |
| packages/studio/src/components/editor/useColorGradingPreviews.ts | New hook for preset/effect preview batching, retries, caching, and animated hover preview frames. |
| packages/studio/src/components/editor/useColorGradingController.ts | Integrates preview capabilities into the existing grading controller API/state. |
| packages/studio/src/components/editor/useColorGradingController.test.ts | Adds tests for preview batching, effect-family requests, preview-only behavior, and animated hover previews. |
| packages/studio/src/components/editor/propertyPanelTypes.ts | Adds MediaOverlayPlacement + AddMediaOverlayHandler types for overlay installs. |
| packages/studio/src/components/editor/propertyPanelPresetPreview.ts | New shared helper to standardize preview/commit pointer/focus handlers for preset cards. |
| packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx | Adds a shared responsive grid class constant for preview card layouts. |
| packages/studio/src/components/editor/propertyPanelFlatOverlaysSection.tsx | New Overlays section that filters tagged Registry components and previews poster/video on hover. |
| packages/studio/src/components/editor/propertyPanelFlatOverlaysSection.test.tsx | Tests overlay filtering, placement derivation, and add-overlay delegation. |
| packages/studio/src/components/editor/propertyPanelFlatEffectsSection.tsx | New Effects section (families, previews, presets, palette + per-effect controls). |
| packages/studio/src/components/editor/propertyPanelFlatEffectsSection.test.tsx | Tests effect families, preview callbacks, applying/removing/resetting effects, and palette behavior. |
| packages/studio/src/components/editor/propertyPanelFlatEffectSpecs.ts | Defines effect families, control specs, and defaults used by the Effects UI. |
| packages/studio/src/components/editor/propertyPanelFlatEffectControl.tsx | Renders per-effect slider/toggle/select controls with correct defaulting/reset behavior. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.tsx | Switches Grade UI to preset cards w/ runtime previews; renames “Strength” → “Amount”; removes legacy mini effects sliders. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx | Updates tests for preset-card preview/commit behavior and removes legacy effects assertions. |
| packages/studio/src/components/editor/PropertyPanelFlat.tsx | Adds new accordion groups for Effects and Overlays; wires preview/overlay handlers; scrolls newly opened group into view. |
| packages/studio/src/components/editor/PropertyPanel.test.tsx | Updates fixed-header/open-body test expectations to account for the new Effects group. |
| packages/studio/src/App.tsx | Wires handleAddMediaOverlay from block handlers into the right panel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed exact head 9b2a1a753a8a159b44b6537e28f6f3be489496c5.
Blocking
packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.tsx:271-274
The Grade preset path rebuilds the entire normalized grading object from only { preset, lut }:
const next = normalizeHfColorGrading({ preset: presetId, lut: grading.lut });That silently drops independently configured effects and palette. The new inspector presents Grade and Effects as composable sibling sections, so this makes the result order-dependent: apply Blur (or a palette) in Effects, then choose a Grade preset, and the prior treatment disappears.
I confirmed this directly against Core: normalizing { preset: "bright-pop", effects: { blur: 0.5 } }, then taking the current preset resolver path for "deep-contrast", changes blur from 0.5 to 0.
Please preserve the currently active Effects/Palette while applying the Grade-owned preset fields (or expose a Core helper that performs that scoped operation), and add a regression starting with a nonzero effect/palette, clicking a Grade preset, and asserting those values survive while the preset changes. Please also audit the Grade reset path for the same whole-object replacement shape.
Important
packages/studio/src/components/editor/propertyPanelFlatEffectSpecs.ts:79-285
This new 200-line Studio registry re-declares effect keys, ranges, defaults/control shape, and palette support that Core already publishes through getHfColorGradingCapabilities(). The values match today, but Studio never consumes that capability API, so future Core additions/range changes can produce a valid serialized treatment that the inspector renders incorrectly or cannot edit. Keep UI-only labels/grouping/formatters here, but derive the canonical keys, bounds, defaults, and palette support from Core. This is a new Studio SSOT seam, separate from the already-reported lint/CLI key copies in earlier PRs.
Ponytail review
propertyPanelFlatEffectSpecs.ts:79-285— join compact presentation metadata against Core capabilities instead of restating the treatment contract — net: -90 lines possible.
Copilot's two preview-generation findings are already covered and not repeated here. Resource lifecycles and the block-installer extraction otherwise looked clean.
jrusso1020
left a comment
There was a problem hiding this comment.
Additive review at 9b2a1a75, weighted to Ular's focus (duplication / deletable / over-complex / core+registry contract match). @Copilot commented (redundant preset-preview renders on mount and on effect-family switch); @magi is reviewing in parallel. Not repeating those or the #2751-#2754 findings.
Good — consumes the shared contracts instead of re-implementing them. The inspector reads getHfColorGradingCapabilities() + core normalize/serialize; blockInstaller.ts delegates the actual install to the existing POST /api/projects/:id/registry/install route (:67) and reuses the existing Studio edit helpers (saveProjectFilesWithHistory, collectHtmlIds, extendRootDurationInSource) for composition insertion — no duplicate install pipeline. DEFAULT_EFFECTS derives from normalizeHfColorGrading("neutral") rather than hard-coding (effectSpecs.ts:283).
Verified — no range bug in the effect sliders (the thing most likely to be one). The bloom master uses max: 300 while core clamps bloom to {0,3}; I traced the mapping — effectsSection.tsx:304/312 displays value*100 and commits next/100, and sub-controls use sliderValue = value*scale / next/scale (effectControl.tsx:34-48), so bloom stores 0–3 and bloomRadius (scale 1, {1,100}) stores its raw range. All match core; the sliders don't overrun the runtime clamp.
Additive (duplication / drift) — effectSpecs.ts is the third consumer that shadows core's effect contract, and the drift surface here is ranges + option counts, not just keys. Same systemic shape as the #2752 lint rule (Magi's blocker) and the #2753 CLI GRADING_KEYS/LUT_KEYS — both already being fixed — so I'm flagging only what's new to this file: effectSpecs.ts hand-codes numeric ranges (bloomRadius {1,100}, bloom master 300 = core-max × 100) and select-option lists (ASCII_STYLES = 8 for core asciiStyle {0,7}; SCREEN_SHAPES = 5 for monoScreenShape {0,4}) that getHfColorGradingCapabilities() already owns. All correct today, but they drift silently if core retunes a range or adds a style. The genuinely UI-specific bits (labels, masterFormat, groupings, master relationships) are legit and should stay. One concrete gap worth closing now regardless of the shared-registry work: the effect keys are satisfies-typed for membership but there's no completeness check, so a new core active effect wouldn't error — it'd just be silently missing from the inspector. An exhaustiveness assertion against HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS (and deriving the select-option counts from the core max) would make this the point where the stack's contract is enforced rather than re-copied. (Inline below.)
Verdict: COMMENT — Ready on the merits; the effectSpecs drift is latent (verified correct today) and best folded into the same core-registry consumption the earlier PRs are getting. Solid, contract-respecting inspector. (Deferring the stamp to the maintainer.)
Review by Jerrai (hyperframes)
jrusso1020
left a comment
There was a problem hiding this comment.
Follow-up to my earlier COMMENT, correcting my verdict: @magi's CHANGES_REQUESTED blocker is real — I reproduced it in source. My prior "ready on the merits" was scoped to the areas I'd audited (effectSpecs / install / slider ranges), not the preset-apply path.
Confirmed — Grade-preset data loss at propertyPanelFlatColorGradingSection.tsx:272. resolvePreset rebuilds grading from only { preset: presetId, lut: grading.lut }, so normalizeHfColorGrading refills adjust/details/effects from the preset — for a Grade preset that zeroes the user's independently-set Effects and Palette (matches Magi's blur 0.5 → 0; it's core's rawX[key] ?? presetX[key] semantics). The diff shows the path is pre-existing (it only dropped the old intensity: 1), but #2755 is the PR that adds independent Effects + Palette to the inspector — so this change is what turns a latent reconstruct into live data loss. In scope.
Additive — the correct shape is already in the same file; it's a one-line consistency break. resolvePreset is the only commit path in this section that doesn't spread the current grading. Every sibling preserves state via { ...grading, … }: intensity (:278), lut (:282), details (:313), adjust (:493). The preset path should match — keep effects + palette, merge the preset's adjust/details onto the current grading — plus Magi's requested regression, and the same audit on the reset path.
Updated verdict: COMMENT, concurring with @magi's REQUEST_CHANGES. My effectSpecs / SSOT + range notes stand as latent cleanups; this preset data loss is the blocker.
Review by Jerrai (hyperframes)
9b2a1a7 to
a473c41
Compare
9b79f5b to
1a12cf4
Compare
a473c41 to
3237565
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review at 323756535. @magi re-reviewing in parallel; independent verdict.
The Grade-preset data-loss blocker is resolved — and the sibling reset path with it.
propertyPanelFlatColorGradingSection.tsxresolvePresetnow preserves the user's independently-set treatment:return resolved ? { ...resolved, effects: grading.effects, palette: grading.palette } : grading. Applying a Grade preset updates adjust/details from the preset while keeping Effects + Palette — no more silent wipe.useColorGradingController.ts:542resetGradinggot the same treatment: it resets the Grade fields to neutral but preserveseffects/palettefromlatestGradingRef.current. That's the sibling path with the identical failure mode, fixed proactively — good catch beyond the exact line flagged.- Regression tests cover both: a preset-apply test asserting
effects:{pixelate:0.5}+ a palette survive abright-popclick, and"resetGrading resets Grade fields without clearing Effects or Palette".
My effectSpecs SSOT-drift finding is also resolved. propertyPanelFlatEffectSpecs.ts now derives control ranges from getHfColorGradingCapabilities() (core) rather than hard-coding them (percent/degrees use controlRange(...); bloom/bloomRadius pull from core), and enumOptions() throws if the ASCII / screen-shape label count doesn't match core's declared range ("${key} labels do not match Core capabilities") — a real completeness guard, not just a membership type. That closes the last of the three re-hardcoding consumers across this stack: lint (#2752), CLI (#2753), and now the inspector all consume the core/parsers SSOT.
CI is CLEAN at 323756535; no failures.
Verdict: COMMENT — the blocker is resolved (plus the reset path), my drift finding is closed, and both paths have regression tests. Ready on the merits. Not stamping (author isn't on the trusted-stamper list); happy to APPROVE on @jrusso1020's go, same as #2751/#2752.
Review by Jerrai (hyperframes)
miguel-heygen
left a comment
There was a problem hiding this comment.
Exact-head re-review at 323756535e96321829cd5c39ced90a492f869c1a.
Audited: the corrective delta in propertyPanelFlatColorGradingSection.tsx and its regression test; preview request guards in both Grade/Effects sections; the Core-capability range/enum derivation in propertyPanelFlatEffectSpecs.ts.
The blocker is resolved at propertyPanelFlatColorGradingSection.tsx:278-281: selecting any Grade preset (including Neutral/reset) now resolves the Grade-owned preset fields while explicitly preserving the existing effects, palette, and LUT. The regression at propertyPanelFlatColorGradingSection.test.tsx:236-281 starts with non-default adjust/details/effect/palette/LUT, applies Bright Pop, and proves the Grade fields change while effect/palette/LUT survive.
The two preview-render comments are also fixed: requests are now gated on missing images and non-loading state (propertyPanelFlatColorGradingSection.tsx:269-276, propertyPanelFlatEffectsSection.tsx:89-112).
The SSOT concern is materially reduced at propertyPanelFlatEffectSpecs.ts:48-99: numeric bounds and enum cardinality now derive from getHfColorGradingCapabilities(), with label-count drift failing immediately. UI-only labels/grouping/formatters remain local.
No new blockers in the corrective delta. Exact-head check-runs are complete with no failures or pending jobs. Local focused execution was unavailable because the isolated worktree could not install dependencies with the host filesystem full; I therefore relied on source-level regression inspection plus exact-head CI and am stating that limitation explicitly.
Verdict: APPROVE
Reasoning: The Grade-preset data-loss path is fixed on the correct ownership boundary with a direct regression, the redundant preview work is guarded, and the canonical range/enum contract now comes from Core.
— Magi
1a12cf4 to
32918fc
Compare
3237565 to
188be7b
Compare
What
Adds selected-media Grade, Effects, and Overlays controls to the flat Studio inspector, with responsive visual grids, shader-rendered look/effect previews, selected-video hover playback, adjustable effect controls, LUT upload, compare, and one-click overlay installation.
Why
Users need to see treatments on the selected media before applying them, while retaining precise manual control and the same persisted contract agents use.
How
The inspector consumes Core capability metadata and the existing runtime bridge. Hover is preview-only; click persists through the existing edit path. Preview batching, cancellation, caching, and synchronous latest-state tracking avoid stale commits and unnecessary renderer work.
Test plan
Stack 5/6. Base: #2754. The separate workspace-layout cleanup is PR 6.