Skip to content

fix(core): make grading writes previewable and honor computed properties - #5157

Merged
miguel-heygen merged 6 commits into
mainfrom
fix/grading-omits-identity-hue-curves
Oct 8, 2026
Merged

miguel-heygen merged 6 commits into
mainfrom
fix/grading-omits-identity-hue-curves

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What

Color grading now produces valid, previewable writes and honors CSS custom properties from computed style:

  1. A single hue curve failed lint. hyperframes media-treatment --grading '{"hueCurves":{"hueVsHue":[...3 points]}}' --apply succeeded, but the composition it wrote then failed hyperframes lint with color_grading_invalid_structure (twice). Studio's grading panel writes through the same function.

  2. A grading property set anywhere but the element's own inline style was ignored without a warning. For example, --hf-color-grading-blur set in a stylesheet or on a parent, with the image staying sharp while saturation and vignette from the same grade showed. On top of that, our docs, skill and CLI capability output told authors to put the animated property's start value inline. Once set, that value overrides the payload's value for that control, so an author who stored a blur and followed the advice pinned it at the start value until a tween moved it.

  3. Dry-run required a write flag. media-treatment --selector X --grading <json> --dry-run --json refused to preview unless --apply was also present. Dry-run now uses the same preparation as apply, reports the exact normalized attribute and color-grading lint verdict, and writes nothing.

Dry-run

JSON output includes attribute: "data-color-grading", value (the exact normalized string or null for removal), and lint with grading findings from the proposed HTML. Both apply and dry-run run the real linter on that prepared HTML and report its color-grading findings. A grade without either --apply or --dry-run still refuses to write.

The command regression fails on main, then passes 21/21 three consecutive runs after the fix. It checks that preview leaves the original bytes unchanged and that apply writes exactly the previewed attribute value with the same lint verdict. The actual source CLI command also returned normalized grading and a successful grading lint verdict without changing the file. CLI typecheck and commit checks pass.

Hue curves

The writer and the validator disagreed about an untouched hue curve:

  • Normalizing fills every hue curve the author did not set with identity, which is an empty list.
  • serializeHfColorGrading wrote the whole hueCurves object as soon as any one curve had values, so the untouched curves went out as "hueVsSaturation":[],"hueVsLuma":[].
  • The contract validator accepts a missing curve but refuses an empty one (a hue curve needs 3 to 16 points).

The serializer now writes only the hue curves that have points. Reading is unchanged: a missing curve normalizes to identity, so the written grading round-trips byte for byte. RGB curves are not affected, since their identity is a valid two-point line.

Files an earlier version already wrote stay fixable. media-treatment --apply validated the stored grading strictly before merging a patch, so a file with empty hue curves refused every later apply, even an unrelated one. It now drops only the three known empty hue curves from the stored grading before merging (an empty curve is what the old writer meant by identity), so the next apply writes the file back valid. Everything else in a stored grading is still checked as before.

Grading properties and the blur report

  • The runtime reads the nine animatable grading properties (--hf-color-grading-blur, -intensity, -exposure and the rest) from the element's computed style instead of only its inline style. A value from a stylesheet or inherited from a parent now applies, as CSS custom properties normally do. Changes to parent styles, classes or stylesheets on a paused page apply on the next seek or play; this PR does not add paused style-change observation.
  • The CLI capability output, the color-grading docs and the media-use skill no longer ask for an inline start value. They now say a set property overrides the payload's value for that control, so set it only on media you animate, starting at the tween's first value, and leave it unset for a static grade.

This closes the report that blur did not render on an image inside a templated sub-composition while saturation and vignette did. That report does not reproduce on main or on 0.8.137. Blur renders the same in the root, in a <template> sub-composition, in a data-composition-src sub-composition and in a sub-composition mounted twice, in both snapshot and render. The two causes that do give that exact symptom on any media are the ones fixed here: a property set outside inline style, and an inline start value overriding a stored blur. A third behaviour also weakens blur: it is a crossfade toward a blurred copy, not a true radius, so at 0.45 about half the edge detail survives. It is left as is, because changing it would change every existing render.

Proof

Real CLI on the reported hue-curve project, media-treatment --selector "#v" --grading '{"hueCurves":{"hueVsHue":[[0,0],[120,20],[240,0]]}}' --apply, then lint:

written hueCurves lint
main {"hueVsHue":[...],"hueVsSaturation":[],"hueVsLuma":[]} exit 1, 2 color_grading_invalid_structure errors
this PR {"hueVsHue":[...]} exit 0, no findings
this PR, on the project as the old writer left it, then an unrelated --grading '{"adjust":{"exposure":0.1}}' --apply {"hueVsHue":[...]} exit 0, no findings (main: the apply itself exits 1)

hyperframes snapshot in Chromium of a checkerboard image whose grade sets only saturation, with --hf-color-grading-blur: 1 in a stylesheet rule. Sharpness is the Laplacian variance of the image area; lower is blurrier:

sharpness
main 40863.5 (blur ignored)
this PR 2.6 (blurred)
this PR, stylesheet value 0 40863.5

Checks

  • New tests, each failing on main:
    • a grading with only hueVsHue serializes that one curve, and the written attribute passes lint;
    • applying a patch over a stored grading with empty hue curves succeeds and writes only the authored curve;
    • the runtime reads --hf-color-grading-blur set in a stylesheet;
    • the CLI's animation advice states the override and no longer asks for an inline start.
  • Original focused proof: colorGrading.test.ts (core and runtime) 93/93 and media-treatment.test.ts 20/20, 3 runs in a row, including the existing byte-identical round-trip and inline-property tests. Core and CLI typecheck pass; the skills manifest is regenerated and in sync.

The repair preserves unknown empty curve keys for the validator to reject. Its public-API regression fails against the broad empty-array filter and passes with the bounded repair. The changed CLI test and CLI typecheck passed on the final head.

Known gate red: Studio timeline viewport gate has the overscan regression introduced by #5109. The approved fix is #5151, owned separately; this PR leaves that work in its existing lane.

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1534 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

  • move-none-pct-r0-root-z50: tracking 0.07, pressJump 0, drop 0, reload 0.07, render 0.02, renderKey -, undo false, teleport true / tracking 0.07, pressJump 0, drop 0, reload 0.07, render 0.02, renderKey -, undo true, teleport true / tracking 0.07, pressJump 0, drop 0, reload 0.07, render 0.02, renderKey -, undo true, teleport true

@mintlify

mintlify Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Oct 8, 2026, 9:36 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@miguel-heygen miguel-heygen changed the title fix(core): write only authored hue curves, so a single hue curve passes lint fix(core): grades pass lint with one hue curve, and grading properties apply from stylesheets Oct 7, 2026
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 7, 2026 06:25
@miguel-heygen miguel-heygen changed the title fix(core): grades pass lint with one hue curve, and grading properties apply from stylesheets fix(core): make grading writes previewable and honor computed properties Oct 7, 2026

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 88883305: approved on the code. GitHub reports this branch as conflicting with main (mergeable_state: dirty). A conflict resolution makes a new head, and this repo needs an approval on the last push, so I'll re-check the merge's own patch when it's re-pinned.

What I checked

  • Hue curves. serializeHfColorGrading now writes only the authored curves through authoredHueCurves. Reading is unchanged, because a missing curve normalizes to identity, so the round-trip stays byte-identical (the existing round-trip test still passes). RGB curves are untouched.
  • The repair is narrow. withoutEmptyHueCurves drops only an empty array under one of the three known keys. An unknown key, or a non-array, still reaches assertKnownGradingShape, and a mutation that drops unknown empties too is caught by the tests.
  • Dry-run. --grading with --dry-run alone now previews, and a grade with neither flag still refuses. Dry-run never writes, and the JSON carries attribute, value and the color-grading lint verdict computed from the prepared HTML. The example drops the old --apply --dry-run pairing.
  • Computed style. readAnimatedValue reads getComputedStyle(...). Nothing in the runtime registers these properties with @property / registerProperty, so an unset property still reads as "" → null, and the payload's value still wins when nothing is set. I grepped the registry, templates and skills: no shipped composition sets a --hf-color-grading-* property on a container, so the new inheritance changes no existing output.
  • Docs and skill. cli.mdx, color-grading.mdx and grading.md now say a set property overrides the payload. media-treatment-recipes.md still authors start values inline, but only for tweens (:215, :694, :754, :918), which matches the new rule.
  • Tests (local, this head): core colorGrading.test.ts and runtime colorGrading.test.ts 93/93, media-treatment.test.ts 21/21. CI is green apart from the conflict.
  • Mutations: 7 of 8 caught. Caught: serializing every curve again, the runtime reading inline style only, no repair, the repair dropping unknown keys, dry-run needing --apply, dry-run writing, and value missing from the JSON. Survived: replacing the reported lint with { ok: true, findings: [] }. Every test grade lints clean, so the verdict is never seen failing. A dry-run test on a payload that lints with a color-grading error would pin it.

Nits

  • readAnimatedGrading calls readAnimatedValue up to nine times per entry per redraw, and each call is now a fresh getComputedStyle(element). That's one style recalc per frame at most, so it's cheap, but reading the declaration once per entry and passing it in would make that explicit.
  • Inherited means a property set on a parent now reaches every graded media under it, not just one. The docs say "on a parent", which is accurate. One more sentence to make the fan-out explicit would help authors who group several clips.
  • --dry-run with no --grading and no --clear still says --apply requires --grading <json>, which names a flag the person didn't pass.

— Rames

@somanshreddy somanshreddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 7281e78f, my first pass on this PR. No blockers; approving.

The merge is code-free. I compared the PR's content at 88883305 (from its merge base) with 7281e78f (from its main parent). The only difference is the regenerated skills-manifest.json hash; every code and docs hunk is identical.

Checked:

  • Hue curves: authoredHueCurves serializes only non-empty curves, so a single hue curve no longer writes the other keys as [].
  • Legacy files: on the CLI side, withoutEmptyHueCurves strips empty known curves from a grading saved earlier before merging, while unknown keys and non-array values pass through to assertKnownGradingShape.
  • Computed style: readAnimatedValue now reads getComputedStyle. The --hf-* properties aren't registered (CSS.registerProperty / @property), so an unset property still computes to "" and returns null. Inheritance from a parent is the intended new behaviour, and the capability rule text now says so.
  • Dry run: --dry-run without --apply validates, lints and reports, and never writes.
  • Lint dependency: @hyperframes/lint is already statically imported elsewhere in the CLI (utils/lintProject.ts), so the new import adds no new dependency.

Lows / nits (none blocking):

  1. Lint ok untested: forcing ok to true (M7) passes all 21 CLI tests. Add a case with an error-severity color_grading_* finding.
  2. Lint can block a write: lintHyperframeHtml runs before writeFileSync, so a lint exception now aborts an --apply that used to succeed. Consider wrapping it and reporting lint: null on failure.
  3. Error message: --dry-run with neither --grading nor --clear says "--apply requires --grading ", which is misleading for a dry run.
  4. Per-frame cost: each readAnimatedValue call makes its own getComputedStyle(element), about 9 per element per frame across the call sites. Style only recalculates once per frame, but hoisting one computed style per entry would be cheaper.

Tests and CI:

  • vitest passes: core colorGrading and runtime colorGrading 93/93, CLI media-treatment 21/21.
  • Mutants: 7 of 8 caught. Caught were inline-only reads, serializing every hue key, not stripping legacy empty curves, the strip filter, dry-run requiring --apply, dry-run writing, and leaking non-grading lint findings. M7 (above) survived.
  • All 11 required checks are green at the head (86 checks succeeded, 2 skipped). It's mergeable, and git merge-tree against current main is clean.

— Somu

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 8dae5ef Oct 8, 2026
95 checks passed
@miguel-heygen
miguel-heygen deleted the fix/grading-omits-identity-hue-curves branch October 8, 2026 15:40

This branch was successfully deployed

1 active deployment
staging - docs — 7281e78f Deployed Oct 8, 2026 by mintlify[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants