feat(studio): add professional grading controls - #2797
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a more “pro” color-grading surface to Studio (scopes + wheels/curves + HSL secondary sampling) while reusing the existing grading controller/preview runtime so grading can be inspected, previewed, and applied consistently with render output.
Changes:
- Introduces new grading UI primitives (transactional numeric input, 3-way wheels, RGB/hue curves, secondary qualifier/correction with eyedropper sampling) and integrates them into the Flat grading section.
- Adds capture-based scopes (histogram/waveform/RGB parade/vectorscope) with debounced capture/refresh behavior (no polling).
- Improves preview/thumbnail generation to be pipeline-order-aware (effects/palette included for preset thumbnails) and supports frame capture for sampling/scopes.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/studio/src/icons/SystemIcons.tsx | Adds Eyedropper/Trash icons used by new grading UI. |
| packages/studio/src/components/editor/useInspectorGestureTransaction.ts | Adds useInspectorGestureDraft for transactional draft state during gesture edits. |
| packages/studio/src/components/editor/useColorGradingScopes.ts | New hook to capture a frame and compute scopes analysis with debounce/refresh. |
| packages/studio/src/components/editor/useColorGradingScopes.test.tsx | Unit tests for scopes capture behavior (open/refresh/unavailable). |
| packages/studio/src/components/editor/useColorGradingPreviews.ts | Extends preview rendering to include effects/palette for preset thumbnails; adds frame capture API. |
| packages/studio/src/components/editor/useColorGradingController.ts | Wires capture API into controller; adjusts persistence gating to “authored values” semantics. |
| packages/studio/src/components/editor/useColorGradingController.test.ts | Updates/expands tests for preview batching/partial results and secondary persistence behavior. |
| packages/studio/src/components/editor/propertyPanelGradingNumberField.tsx | New transactional numeric text field for grading controls. |
| packages/studio/src/components/editor/propertyPanelGradingNumberField.test.tsx | Tests for the new numeric field (begin/preview/settle/cancel semantics). |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.tsx | Major Flat grading UI expansion: scopes, primary sliders, wheels, curves, secondary, LUT actions refactor. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingSection.test.tsx | Updates and adds coverage for new UI ordering and sampling behavior. |
| packages/studio/src/components/editor/propertyPanelFlatColorGradingAccessory.tsx | Extracts the Flat grading accessory (compare/status/reset) into its own component. |
| packages/studio/src/components/editor/PropertyPanelFlat.tsx | Passes captureGradedFrame through to the Flat grading section. |
| packages/studio/src/components/editor/propertyPanelColorWheels.tsx | New 3-way color wheels component with pointer/keyboard interactions and transactional commit. |
| packages/studio/src/components/editor/propertyPanelColorWheels.test.tsx | Tests wheels hue mapping, rendering direction, and preview vs commit behavior. |
| packages/studio/src/components/editor/propertyPanelColorSecondary.tsx | New secondary (HSL) qualifier/correction UI with captured-frame sampling + matte visualization. |
| packages/studio/src/components/editor/propertyPanelColorSecondary.test.tsx | Tests adding/removing secondaries, bypass, range ordering, capture error handling, keyboard sampling. |
| packages/studio/src/components/editor/propertyPanelColorScopes.tsx | New scopes UI (mode tabs + canvas rendering + refresh). |
| packages/studio/src/components/editor/propertyPanelColorGradingControls.tsx | Refactors existing controls to share slider specs and grading actions/helpers. |
| packages/studio/src/components/editor/propertyPanelColorCurves.tsx | New curves UI wrapper (tabs + graph + numeric editing + reset/delete). |
| packages/studio/src/components/editor/propertyPanelColorCurves.test.tsx | Tests curve tabs, seam behavior, and commit semantics. |
| packages/studio/src/components/editor/propertyPanelColorCurveGraph.tsx | New curve graph implementation (pointer + full keyboard model) for RGB and hue curves. |
| packages/studio/src/components/editor/colorGradingFrameAnalysis.ts | New frame analysis utilities (scope analysis, pixel decode, matte build, secondary sampling). |
| packages/studio/src/components/editor/colorGradingFrameAnalysis.test.ts | Unit tests for scope endpoints, alpha weighting, circular hue averaging, and matte behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adc63c8 to
6595ef3
Compare
e82f474 to
96b67cf
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 96b67cfc as the studio leg of the whole trio (#2796 core → #2798 CLI → #2797 studio), not in isolation — the findings below are mostly only visible that way. I verified the runtime-facing claims by running them rather than reading them, which is what turned one of them from a suspicion into a reproduction.
Additive only. @Copilot's two stale-scope comments are fixed in e82f4745 and covered by tests (useColorGradingScopes.ts:29,35 clears prior analysis on close and at capture start; useColorGradingScopes.test.tsx 4/4 pass) — not restating them. Magi is reviewing the same stack in parallel.
Audited (read end-to-end): colorGradingFrameAnalysis.ts, propertyPanelColorSecondary.tsx, propertyPanelColorWheels.tsx, propertyPanelColorCurveGraph.tsx, useColorGradingPreviews.ts, useColorGradingScopes.ts, useInspectorGestureTransaction.ts, propertyPanelFlatColorGradingSection.tsx, plus core/src/runtime/colorGrading.ts (GLSL + texture packing) and core/src/colorGrading.ts capabilities on the other side of the boundary.
Trusting: the scope-drawing canvas code in propertyPanelColorScopes.tsx (read structure, did not verify pixel output), SystemIcons.tsx.
First: this PR's CI never ran its own tests, and that includes the gate the body cites
CI is gated pull_request: branches: [main] (ci.yml:22-23). This PR's base is feat/agent-native-color-grading, so CI never fires — only regression, preview-regression and Player perf ran at this head, and the PR reads CLEAN and mergeable off those. Same gap as #2798.
Worth naming specifically: the body says "Keep all production components below Studio's 600-line architecture gate." That gate is ci.yml:673-704, inside the skipped workflow. The claim is true — I checked every changed non-test file, largest is PropertyPanelFlat.tsx at 575 — but nothing enforced it here.
So I ran the skipped set locally at this head:
| Gate CI skipped | Result |
|---|---|
| Studio grading/gesture focused tests | 76 pass (body said 75) |
| Full studio suite | 2811 pass, 18 todo, 0 fail (body said 2,809 + one order-leak failure — no failure reproduced here) |
tsc --noEmit (studio) |
clean |
oxlint . |
0 warnings, 0 errors |
oxfmt --check (changed dirs) |
clean |
check:package-subpaths / workspace-contracts / package-cycles |
✓ (no new cycle) |
gen-skills-manifest --check / check-skill-mirror |
✓ |
| file-size gate (600) replicated on this PR's studio files | ✓ none over |
All green. Reporting that as verification, not as a substitute for CI running it.
blocker — the matte and eyedropper use HALF the saturation/luma softness the render uses
colorGradingFrameAnalysis.ts:95,101:
softRangeMask(hsv.saturation, key.saturation.min, key.saturation.max, key.saturation.softness * 0.5)
softRangeMask(rec709Luma(...), key.luma.min, key.luma.max, key.luma.softness * 0.5)That * 0.5 was copied from the shader (runtime/colorGrading.ts GLSL :1075-1076) — but the shader's * 0.5 exists to undo the pack-time / 0.5 at runtime/colorGrading.ts:2725,2731:
secondary.key.saturation.softness / 0.5, // packed into an RGBA8 texel (:2792-2795, unitFloatToByte)The two cancel, so the runtime's effective softness is softness, not softness * 0.5. The JS port applies the read-side scale without the pack-side one and lands at half.
Measured, not argued. Pixel rgb(255,204,204) (HSV sat = 0.2), key saturation {min 0.5, max 1, softness 0.4}, hue/luma wide open:
saturation axis studio matte 0.000000 runtime 0.150772 matte byte 0 vs render byte 38
luma axis studio matte 0.000000 runtime 0.196538
hue axis studio 0.798376 runtime 0.796850 <- control: agrees
So a pixel the render feathers in at ~15% reads as completely unselected in the matte. The hue axis agreeing is the control — it rules out my simulation being the thing that's wrong, and it localises the bug to the two axes that go through the /0.5 pack.
Fix is two lines — drop both * 0.5. Verified:
after fix saturation studio 0.156250 runtime 0.150772 (byte 40 vs 38 = 8-bit texel quantization, expected)
luma studio 0.196538 runtime 0.196538 (exact)
Grading this a blocker rather than a nit because the matte is the verification surface this PR exists to add — the body's stated purpose is "manual review, before/after comparison, and agent-output verification." A selection preview that disagrees with the render on the softness axes undercuts the feature's whole reason for being, and the softness sliders let you author right into the broken range (propertyPanelColorSecondary.tsx:356-370 allows softness to 0.5; the matte can never show more than 0.25 of feather).
important — the test named after this convention can't detect it, in either direction
colorGradingFrameAnalysis.test.ts:68 is called "matches the shader's half-softness edge convention" and pins nearly the same key I used above (saturation {0.5, 1, 0.4}). It encodes the misreading: there is no half-softness convention in the shader, there's a /0.5 pack and a *0.5 unpack that cancel.
I applied the correct two-line fix and re-ran:
colorGradingFrameAnalysis.test.ts 6/6 pass (unchanged)
src/components/editor/ (whole dir) 823 pass (unchanged)
So the suite is green with the bug and green with the fix — 823 tests that cannot distinguish the correct softness from double it. belowOuterEdge uses a pure grey (saturation exactly 0), which sits below min − softness at either value, so the assertion is satisfied by construction.
This is the shape #2798 — the PR directly beneath this one — just fixed for the vendored analyzer with mediaGradeAnalyzer.vendoredParity.test.ts. Here the duplication is unavoidable (GLSL can't be imported into a React panel), which makes the parity guard more necessary, not less. Concrete remedy, mirroring #2798's own fix: move the scalar mask into packages/core next to the GLSL it mirrors, and pin it with a test that runs sample triples through the JS mask and asserts against the pack → unitFloatToByte → shader-decode round-trip. A unit test that asserts a belief about another system is not a guard on that system.
important — studio is a fourth bounds layer that doesn't derive from the SSOT, and the prior art is in this same stack
#2796 added COLOR_GRADING_ADVANCED_LIMITS (parsers/src/colorGradingContract.ts:5-14) and wired all three existing layers to it — validation, core capabilities, core clamps. Zero studio files reference it, and the new panels hardcode every bound instead.
Two things make this more than a style preference:
- The CLI leg of this stack already derives from core:
media-treatment.ts:34,85,567all callgetHfColorGradingCapabilities(). - Studio's own sibling Effects panel already derives from core:
propertyPanelFlatEffectSpecs.ts:49, forty lines from the new files.
And getHfColorGradingCapabilities() already exposes every bound the new panels hardcode (colorGrading.ts:980-1043) — wheels.controls.{hue,amount,level}, secondaries.hue.{center,range,softness} plus rangePlusSoftnessMax, secondaries.{saturation,luma}.softness, secondaries.correction.*. So this is a swap, not a new abstraction.
I checked each hardcoded value against the table and they all agree today — 180 (propertyPanelColorSecondary.tsx:279,290,300), max={50} → 0.5 (:361), ±180 / ±100 (:379-383). So it's drift risk, not a live bug, with one exception below.
important — hue-center has three different behaviours for one contract bound
Same parameter, same feature, three answers:
| Site | Behaviour |
|---|---|
propertyPanelColorSecondary.tsx:265 |
slider max={359}, no wrap |
propertyPanelColorWheels.tsx:222 |
number field max={359.99}, and wrapHue on drag/keys (:20-22,41,63-65) |
colorGrading.ts:1016-1017 (contract) |
min: 0, maxExclusive: 360, wrap: true |
An agent writing through the CLI can author hue.center: 359.7 legally; open it in studio's secondary panel and it silently clamps to 359, and there's no wrap where the contract says there is. Deriving from capabilities.secondaries.hue.center fixes this and the item above together.
nit — refreshKey re-serializes the whole grading on every render
propertyPanelFlatColorGradingSection.tsx:209 — refreshKey={serializeHfColorGrading(grading)}, unmemoized, two lines below resolvedGrading and secondaryInputGrading which are both useMemo'd. It's only a nit because the drag path previews through the runtime rather than through grading, so this doesn't run per pointermove — but it's a free useMemo and it's inconsistent with its immediate neighbours.
What's genuinely strong here
Three things I checked specifically because they're the easy ones to get wrong, and they're right:
-
The processing-order handling is correct, and I verified it against the GLSL call order rather than the comment.
secondaryInputGrading(propertyPanelFlatColorGradingSection.tsx:143-160) reconstructs exactly{intensity: 1, adjust, wheels, curves, hueCurves, colorSpace}and feeds that to the capture at:367. That is precisely the runtime's pre-secondary prefix —applyPrimaryGrade → applyTonalWheels → applyRgbCurves → applyHueCurves(GLSL:1091-1094,:1153-1155) — and it correctly excludes the LUT, which the runtime applies after secondaries (:1156), plus effects/palette. Sampling the fully graded frame here would have made the eyedropper self-referential and the matte drift as you tuned the correction. It doesn't. That's the hard part of this feature and it's exactly right. -
Real reuse where reuse is possible.
propertyPanelColorCurveGraph.tsx:3-12,93,96importscompileHfColorCurve/compileHfHueCurve/HF_COLOR_CURVE_MAX_POINTSfrom core, so the curve you see on screen is drawn with the same monotone interpolation the runtime samples — no second interpolator. Secondary count comes fromCOLOR_GRADING_MAX_SECONDARIES. The two findings above are about finishing that instinct, not introducing it. (Incidentally: my #2796 note about the unusedcompileHfColorCurvebarrel export removal doesn't break this — they're still exported fromcolorGrading.ts:20-24, so the./color-gradingsubpath studio imports from still carries them. Checked, since it was my own finding.) -
useInspectorGestureDraftis cleanly additive. I audited every consumer of the module —propertyPanelCommitField.tsx:37,propertyPanelColor.tsx:205, plus the new wheels/curves — and the existinguseInspectorGestureTransactionsignature is untouched, so there's no contract change for the two pre-existing callers.
Verdict
Verdict: REQUEST CHANGES
Reasoning: The softness divergence is a reproduced two-line correctness defect in the exact surface this PR exists to provide, and the test named after that convention is green in both directions, so nothing downstream will catch it. Everything else — the pipeline-order handling especially — is solid work, and the SSOT/hue-center items are cheap to fold into the same pass. Not a stamp either way: that's James's call, not mine.
— hyperframes
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 96b67cfc4715953098ded2b67a98852ae71654b5 additively after reading the existing feedback. The two Copilot scope-lifecycle threads are resolved at this head. Rames's exact-head CHANGES_REQUESTED review already covers the matte-softness mismatch, missing parity guard, capability-bound/hue-center drift, and the skipped full Studio CI caused by this PR targeting a stacked base rather than main; I am not repeating those points.
-
[Blocker] Copying a grade to file/project scope uses pixel-active semantics at
packages/studio/src/components/editor/useColorGradingController.ts:523-528, while ordinary persistence correctly uses authored-value semantics at:480-482. Core explicitly preserves a keyed disabled secondary as authored but inactive (packages/core/src/colorGrading.ts:1490-1522), and the existing regression only covers ordinary persistence (useColorGradingController.test.ts:248-268). As written, Apply to Scope serializes that grade asnulland drops the authored qualifier. UsehasHfColorGradingAuthoredValueshere and add a disabled-secondary scope-copy regression. -
[Blocker] Reset omits the current LUT while preserving Effects and Palette (
useColorGradingController.ts:551-558), and the new test pinslut: null(useColorGradingController.test.ts:569-589). That contradicts the PR body's explicit promise to preserve independent user LUT state when resetting grading. Preserve the current LUT and update the regression accordingly. -
[Important] Curve deletion can be overwritten by settlement of an overlapping keyboard gesture. Arrow movement begins/previews a transaction (
propertyPanelColorCurveGraph.tsx:344-352), Delete commits directly without cancelling it (propertyPanelColorCurves.tsx:56-67), and the matching ArrowUp later settles the stale transaction (propertyPanelColorCurveGraph.tsx:475-478;useInspectorGestureTransaction.ts:35-45). Cancel the active transaction before committing deletion and cover ArrowDown → Delete → ArrowUp.
The exact-head checks that did run are terminal green. As Rames noted, the stacked base means the repository's full Studio CI workflow did not run; his local full-suite/typecheck/lint/format/contract verification supplies the missing evidence for this review.
Verdict: REQUEST CHANGES
Reasoning: Existing feedback already blocks on render/matte parity. Additively, Apply to Scope drops authored data, reset violates the promised LUT-preservation contract, and overlapping curve gestures can restore a deleted point.
— Magi
be791e3 to
20f4bde
Compare
96b67cf to
794930a
Compare
|
Addressed the complete new review set in |
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 794930a07568deddd55ba0d891b68bec739641fa against every prior finding and the full fix delta.
All prior blockers are resolved:
- The matte/eyedropper no longer carries a second softness implementation.
colorGradingFrameAnalysis.tsdelegates to the shared corecalculateHfColorGradingSecondaryMask, whose saturation/luma softness uses the full authored value. Core now pins the runtime texture-pack/decode convention, and Studio pins both feather edges. - Studio wheels and secondaries derive ranges, identities, wrapping, and correction limits from
getHfColorGradingCapabilities()rather than maintaining a fourth bounds table. The legal359.7°hue-center case is regression-covered. - Apply to Scope now uses
hasHfColorGradingAuthoredValues, with a disabled-secondary scope-copy regression. - Reset preserves LUT, Effects, and Palette, with the expected contract pinned.
- Curve deletion cancels the active transaction before committing, and ArrowDown → Delete → ArrowUp is regression-covered.
- The scopes refresh serialization is memoized.
Fresh delta audit found no replacement blocker or unnecessary abstraction. Moving the qualifier mask into core reduces duplication and gives Studio a single product-level implementation while leaving the unavoidable GLSL mirror explicitly guarded.
Verification at the exact head:
- Core color-grading tests: 36/36
- Focused Studio grading/controller tests: 35/35
- Core typecheck: pass
- Studio typecheck: pass
- Oxlint on the touched Core/Studio files: 0 warnings, 0 errors
- Exact-head GitHub checks: terminal green
Verdict: APPROVE
Reasoning: Every prior correctness and SSOT finding is fixed with targeted regression coverage, the replacement design is simpler, and fresh exact-head verification found no new blocker.
— Magi
jrusso1020
left a comment
There was a problem hiding this comment.
Re-reviewed at 794930a0. All four of my findings are fixed, and the blocker took the stronger option — the math moved into core rather than being patched in place. My blocking concerns are resolved on the merits.
Not re-verifying @miguel-heygen's three (Apply-to-Scope authored semantics, reset/LUT preservation, curve-delete gesture race) — it's re-reviewing those itself and has approved at this head. Mine below.
The blocker is fixed, and I verified it independently rather than by reading the diff
calculateHfColorGradingSecondaryMask now lives in packages/core/src/colorGrading.ts:1132-1152, next to the GLSL it mirrors and with a comment saying so, and colorGradingFrameAnalysis.ts:59-71 is a thin adapter that does RGB→HSV + Rec.709 luma and delegates. One owner for the math. The * 0.5 is gone repo-wide.
Two things I checked beyond that it "looks right":
The new mask agrees with the runtime. I re-ran my independent simulation of the pack → unitFloatToByte → GLSL-decode path against the real core function at this head:
saturation core 0.156250 runtime-sim 0.150772 delta 0.005478 (= 8-bit texel quantization)
luma core 0.156250 runtime-sim 0.150772 delta 0.005478
hue core 0.500000 runtime-sim 0.500000 delta 0.000000
Also worth noting the hue branch is now a closer port than before — it mirrors the GLSL hueRangeMask structure directly (:1139-1146) instead of routing through the generic softRangeMask, and it drops the redundant min(softness, 180 − range) clamp that the normalizer already guarantees.
Both new guards actually catch a reintroduction — mutation-verified, not assumed. I put the * 0.5 back into calculateHfColorGradingSecondaryMask and re-ran:
core/src/colorGrading.test.ts FAIL "saturation: expected 0 to be greater than 0" (baseline 36/36)
studio colorGradingFrameAnalysis.test.ts FAIL "expected +0 to be close to 0.15625" (baseline 6/6)
I expected the core test's parity assertion to be insensitive, since toBeCloseTo(shaderMask, 2) compares the same function against itself with two softness values and so only measures byte-quantization tolerance. It's the toBeGreaterThan(0) liveness assertion on the same test that does the catching. So the guard works; the thing to know is which assertion is load-bearing — if a future fixture change makes productMask > 0 at half softness, that protection is gone. The studio-side absolute pin (toBeCloseTo(0.15625, 5), colorGradingFrameAnalysis.test.ts:80-81) is the more durable of the two, and it uses the exact pixel from the original repro. Renaming the old test away from "half-softness convention" also removes the misleading premise.
The other three
- SSOT derivation — done properly.
propertyPanelColorSecondary.tsx:19andpropertyPanelColorWheels.tsx:19now holdgetHfColorGradingCapabilities()slices, and every bound reads from them: hue center min, range, softness withrangePlusSoftnessMax(:315,327-328),secondaries.max(:106,150), per-axis capability lookup (:342), correction limits (:417), and the wheels' identity values for reset. Studio is no longer a fourth hardcoded layer. - hue-center — consistent now, and both panels wrap:
wrapHueCenter(propertyPanelColorSecondary.tsx:24-27) andwrapHue(propertyPanelColorWheels.tsx:28-32), each computing its span frommin/maxExclusiverather than a literal.359.7round-trips. refreshKeynit — memoized asscopesRefreshKey(propertyPanelFlatColorGradingSection.tsx:160).
One small leftover, nit only: wrapHueCenter and wrapHue are now byte-identical functions in two files, and the epsilon is RANGE_GAP = 0.01 in one and a bare 0.01 in the other. A single exported helper next to the contract would close the last seam. Behaviour is correct either way.
Gates re-run at this head (CI still skips them)
Base is still feat/agent-native-color-grading, so CI still never fires — regression, preview-regression, Player perf all terminal green, and the PR reads CLEAN off those.
| Gate | Result |
|---|---|
| studio full suite | 2814 pass, 18 todo, 0 fail (was 2811 — 3 new) |
| core full suite | 1411 pass |
parsers colorGradingContract.test.ts |
6 pass |
tsc --noEmit studio / core / parsers |
clean |
oxlint . |
0 warnings, 0 errors |
oxfmt --check on this PR's 24 changed files |
clean |
| subpaths / workspace-contracts / package-cycles | ✓ |
| skills-manifest / skill-mirror | ✓ |
| 600-line file-size gate replicated | ✓ none over |
Verdict
Verdict: COMMENT — my blockers are cleared
Reasoning: All four findings fixed, the blocker fixed at the right layer, and both new regression guards mutation-verified to catch a reintroduction. I'm not holding a block on the merits. Leaving this as COMMENT rather than an approval because clearing my earlier CHANGES_REQUESTED requires a stamp that isn't mine to give — @jrusso1020 can greenlight or dismiss it. The PR is mechanically CLEAN, so the stale state isn't gating the merge button.
— hyperframes
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-stamp after the base update at exact head 794930a07568deddd55ba0d891b68bec739641fa.
The PR head/tree is byte-identical to my previously approved exact head. The effective diff is now the 24-file Studio surface on top of merged #2798, with no new production or test content introduced by the base update.
Audited: current base/head topology and effective 24-file diff; every prior fix site and its regression coverage in the Core mask, Studio capabilities/wrapping, scope lifecycle, controller scope-copy/reset paths, curve transaction cancellation, and scopes refresh memoization.
Trusting: unchanged feature implementation already reviewed at this identical tree; there is no new line-level delta to reassess.
All prior findings remain resolved:
- Studio matte/eyedropper analysis still delegates to Core's shared secondary-mask implementation, with runtime pack/decode and feather-edge coverage.
- Wheels and secondaries still derive bounds, identities, and wrapping from the shared capabilities contract.
- Apply to Scope preserves authored disabled secondaries; reset preserves LUT, Effects, and Palette; curve deletion cancels an active keyboard transaction.
- Scope state clears stale analysis, redraws empty state, and memoizes its refresh serialization.
Both review threads are resolved. Fresh exact-head verification shows 52 check runs terminal with 44 successes and eight intentional skips, including all eight regression shards, Windows render/tests, Studio load smoke, global-install smoke, build, typecheck, lint, format, and unit/runtime-contract suites.
Verdict: APPROVE
Reasoning: The base update introduced no content delta at the reviewed head, every previously verified correctness and SSOT fix remains present, and all applicable exact-head CI is terminal green.
— Magi
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at 794930a07568deddd55ba0d891b68bec739641fa to clear my stale CHANGES_REQUESTED, which was pinned to the superseded commit 96b67cfc4. Doing this on @jrusso1020's explicit go.
My blockers were resolved at this head and I detailed the verification in review 4781325396. Recapping only what the approval rests on:
- The secondary qualifier moved into core as
calculateHfColorGradingSecondaryMask, next to the GLSL it mirrors, andcolorGradingFrameAnalysis.tsis now a thin RGB→HSV adapter. That function is onmainas of #2796 (packages/core/src/colorGrading.ts:1132), with nosoftness * 0.5reintroduced anywhere. - I mutation-verified both new guards actually catch a reintroduction of the halving, rather than grading them by reading them. Core fails
expected 0 to be greater than 0; studio failsexpected +0 to be close to 0.15625, using the exact pixel from the original repro. - Both panels derive their bounds from
getHfColorGradingCapabilities(), hue center wraps offmin/maxExclusivein both, andrefreshKeyis memoized.
Re-confirmed at this head before stamping:
- Head unchanged, and the diff is still the same 24 files, all under
packages/studio. The base change moved #2796's core files ontomainrather than pulling anything new in. - The reparent onto
mainun-skipped theCIworkflow, which never fired once while this PR was stacked. Tests, Typecheck, Lint, Format, theFile size checkthat the PR body cites, and Windows render verification are now green in CI itself rather than only in my local replication. Zero non-success check-runs at this head.
One nit stands, unchanged and non-blocking: wrapHueCenter and wrapHue are byte-identical in two files, with the epsilon as RANGE_GAP in one and a bare 0.01 in the other. A single exported helper next to the contract would close the last seam.
Not merging. That's @jrusso1020's call.
— hyperframes
What
Why
The advanced runtime needs a direct, inspectable Studio surface for manual review, before/after comparison, and agent-output verification.
How
Test plan
heygen-showcaseselection, scopes, edit, Undo, playback, and render-parity checksStack: 3/3. Base:
feat/agent-native-color-grading. Merge after #2798.