Skip to content

feat(core): define media treatment capabilities#2751

Merged
ukimsanov merged 1 commit into
mainfrom
feat/media-treatment-schema
Jul 24, 2026
Merged

feat(core): define media treatment capabilities#2751
ukimsanov merged 1 commit into
mainfrom
feat/media-treatment-schema

Conversation

@ukimsanov

Copy link
Copy Markdown
Collaborator

What

Defines the Core media-treatment contract for real <img> and <video> elements: normalized grading, LUT, finishing, effect, preset, capability, and animation metadata.

Why

Studio, runtime, render, and agents need one typed source of truth instead of parallel effect lists or ad hoc payloads.

How

Extends the existing color-grading schema while preserving shipped preset IDs and persisted data-color-grading compatibility. Canonical key registries drive validation and capability discovery.

Test plan

  • Core color-grading tests added/updated
  • Core typecheck, format, lint, tracked-artifact, file-size, and Fallow gates pass
  • Manual testing performed
  • Documentation updated (added in stacked PR 3)

Stack 1/6. This PR has no dependency on later PRs.

@ukimsanov
ukimsanov marked this pull request as ready for review July 24, 2026 03:42
Copilot AI review requested due to automatic review settings July 24, 2026 03:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Defines and exports an expanded core data-color-grading contract to support richer “media treatment” metadata (presets, effects, palettes, LUT constraints, and animatable properties) so Studio/runtime/render/agents share a single typed source of truth.

Changes:

  • Expanded color grading schema with canonical key registries, effect families + defaults, palette support, and a capabilities catalog (getHfColorGradingCapabilities).
  • Added new preset families (grade vs effect presets) and “apply defaults” payloads for active effects.
  • Updated/extended core tests to cover new presets, palettes, effect normalization, and capabilities output.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/core/src/index.ts Exports new color grading constants/types and the capabilities accessor from core.
packages/core/src/colorGrading.ts Extends the grading schema (effects, palettes, presets, limits) and adds capability discovery metadata for downstream consumers.
packages/core/src/colorGrading.test.ts Adds coverage for new presets/effects/palettes/capabilities and updates normalization/serialization expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/src/colorGrading.ts
Comment thread packages/core/src/colorGrading.test.ts Outdated

@miguel-heygen miguel-heygen 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.

Audited all 3 changed files at exact head 6703b7c5: normalization/serialization, preset partitioning, palette/LUT bounds, effect activation/defaults, exported capability metadata, and the public index surface. The contract is internally coherent and its canonical key registries drive the derived types/capabilities.

The existing lint-parity test comment is valid but non-blocking for this contract PR: at this head the test is vacuous because color_grading_* findings are introduced only in the runtime PR above it. Please make that parity assertion real in the stack so future key additions cannot drift silently. The nullable input-type observation for palette is also worth aligning with the normalized/runtime JSON contract, but it does not break current authored JSON normalization.

CI is green across build/lint/typecheck/unit/runtime/Windows/CodeQL/CLI/global-install; regression shards were still running when I stamped.

Verdict: Approve.

Reasoning: The core schema and capability catalog are correctly derived from shared defaults and preserve legacy preset compatibility. Remaining comments are type/test-contract hardening, not present-tense correctness blockers.

— Magi

@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.

Additive review at 6703b7c5@magi has APPROVED and @Copilot commented (palette input-type nullability + the currently-vacuous lint-parity test). Layering the render/determinism angle on top; not repeating those.

Verified — the backward-compat claim holds. The body says it "preserv[es] shipped preset IDs and persisted data-color-grading compatibility." Confirmed against the diff:

  • Every shipped preset ID is retained — the old HfColorGradingPresetId union ended at deep-contrast; the new IDs (creator-camcorder, vhs-playback, home-movie-8mm, editorial-halftone, two-ink-print) are all appended, none removed or renamed.
  • The one behavior change in normalizeHfColorGradingintensity now defaults to preset?.intensity ?? 1 instead of a hard 1 (colorGrading.ts:933) — is compat-safe: the only presets whose intensity ≠ 1 are the new ones (creator-camcorder / home-movie-8mm at 0.72). No persisted attribute can carry a new preset, so every existing data-color-grading re-normalizes identically.

Verified — serialization is deterministic and persistence-stable (matters for the stack's "same pixels" thesis). serializeHfColorGrading (colorGrading.ts:953) re-normalizes and bakes preset values into concrete adjust/details/effects numbers, emitted in canonical registry order (the reduce over HF_COLOR_GRADING_*_KEYS + stable insertion order). Two useful properties fall out: (a) the serialized attribute is byte-stable across re-serialization; (b) persisted projects are immune to future preset retuning, because on re-read rawAdjust[key] ?? presetAdjust[key] (colorGrading.ts:907) always finds the baked raw value first.

Concur (minor), already noted by others: Copilot's palette input/normalized nullability mismatch (HfColorGrading.palette: readonly string[] vs NormalizedHfColorGrading.palette: … | null) is a type-only cosmetic inconsistency, not a runtime bug. The lint-parity assertion in colorGrading.test.ts is vacuous until #2752's media rule emits color_grading_* codes, as @magi noted — worth tightening there.

Verdict: COMMENT — Ready on the merits. Clean, well-tested contract expansion; I concur with @magi's approve. (Deferring the approving stamp to the maintainer.)

Review by Jerrai (hyperframes)

@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.

Re-review at 963e1ea6 (was 6703b7c5). @magi has deferred to my verdict, so this is a standalone re-review rather than additive.

Previously-raised concerns — all resolved. ✅ The stack's root SSOT problem (core didn't export top-level/LUT registries, forcing lint/CLI/inspector to re-hardcode) is fixed at the source: packages/core/src/index.ts now exports HF_COLOR_GRADING_TOP_LEVEL_KEYS + HF_COLOR_GRADING_LUT_KEYS, and the key arrays are imported from a single lower-level owner — the new @hyperframes/parsers/color-grading-contract (colorGradingContract.ts, with validateColorGradingContract + the canonical key/limit registries). That module is exactly what #2752's lint rule now consumes, so Magi's "second owner of the schema" blocker is structurally gone. Nice fix — going one level below core to parsers also avoids the core↔lint import cycle.

🚫 Blocker — the preset removal breaks a released consumer (silent neutral output). This revision deletes four shipped preset IDs — natural-lift, fresh-pop, warm-clean, cool-clean — and removes the LEGACY_GRADE_PRESET_IDS retention path (colorGrading.ts, GRADE_PRESETS filter ~L582). My prior review noted these were retained-but-hidden for back-compat; this revision removes them outright.

For persisted projects this is safe — serializeHfColorGrading bakes resolved adjust/details/effects into the attribute and normalizeHfColorGrading prefers raw baked values (rawAdjust[key] ?? presetAdjust[key]), so old attrs keep their look and the preset string just becomes a harmless dangling label.

But the released media-use skill still references these IDs and emits them as bare preset references:

  • skills/media-use/scripts/lib/lut-preset-provider.mjs:18-30CORE_PRESET_IDS still lists all four (under the comment "Keep this list in lockstep with the core runtime contract" — now violated), and PRESET_SYNONYMS (:38-56) maps natural-language phrases to them ("natural lift", "fresh pop", "warm clean", "cool clean").
  • skills/media-use/scripts/resolve.mjs:668-682resolveGrade("natural lift")matchColorLook{kind:"preset", preset:"natural-lift"}mergeSmartAdjust({preset:"natural-lift", intensity:1}), which (without --for) writes a bare {preset:"natural-lift", intensity:1} grading.
  • After this PR, core normalizes that to adjust: all-zero, effects: defaults, and isHfColorGradingActive returns false → the grade is inactive → the user asked for "natural lift" and gets an ungraded frame. Silent wrong output on a shipped command (color-grade resolve landed in #2041, released through v0.7.70).

Same removal drifts the docs: docs/concepts/variables.mdx:90-91 and docs/guides/color-grading.mdx:94 still show natural-lift as an example/default.

Fix: apply the decision in lockstep — either restore the four presets (keep the prior hidden-but-resolvable path), or complete the removal (drop them from CORE_PRESET_IDS + PRESET_SYNONYMS + the docs, and decide what "natural lift"-style intents map to). As-is, #2751 ships a regression the moment the skill requests one of these looks.

nit — the parity guard catches extra keys but not missing ones. The COLOR_GRADING_*_KEYS satisfies readonly HfColorGrading…Key[] re-exports reject invalid keys, but a key present in the type / EFFECT_DEFAULTS yet omitted from the contract array would pass satisfies and then be silently un-settable in both normalize and lint (consistently — so no false lint error, but a real dead control with green CI). The old Object.keys(EFFECT_DEFAULTS) derivation was inherently complete; consider a test asserting COLOR_GRADING_{ADJUST,DETAIL,EFFECT}_KEYS equals Object.keys({ADJUST_ZERO, DETAIL_DEFAULTS, EFFECT_DEFAULTS}) so the single owner can't drift from the runtime defaults.

CI: no failing checks at 963e1ea6; typecheck / build / regression-shards / windows still running at review time. (Note: @magi's earlier APPROVE was at 6703b7c5, before the preset removal.)

Verdict: REQUEST CHANGES
Reasoning: Every prior blocker is resolved and the SSOT root-fix is exactly right, but removing four released preset IDs without updating the media-use skill's mirrored list + synonyms (and docs) turns real user intents into silent neutral output.

Review by Jerrai (hyperframes)

@mintlify

mintlify Bot commented Jul 24, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated (UTC)
hyperframes 🟢 Ready View Preview Jul 24, 2026, 3:59 PM

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

@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.

Re-review at 70213c5a. The blocker is fully resolved — clearing my REQUEST_CHANGES on the merits (holding the formal APPROVE for James's go; see verdict).

The preset removal is now completed in lockstep across every site I flagged:

  • skills/media-use/scripts/lib/lut-preset-provider.mjsCORE_PRESET_IDSRESOLVABLE_PRESET_IDS (with an honest comment: "presets this resolver can safely emit at this stack layer"), the four dead IDs dropped, and — nicely — their synonyms remapped to surviving presets rather than lost: "natural lift"→soft-boost, "fresh pop"→bright-pop, "warm clean"→warm-daylight, "cool clean"→clean-studio. So grade "natural lift" now resolves to a valid nearby look instead of the silent neutral frame it produced before.
  • Regression test added (lut-preset-provider.test.mjs) pinning exactly that: matchColorLook("natural lift").preset === "soft-boost" etc. And the old strict "mirror core's full union" parity test was correctly relaxed to a subset check (RESOLVABLE_PRESET_IDS filtered against the core IDs must be empty) — that's the right contract: the resolver emits a subset and must never emit an ID core lacks.
  • Docs updated: docs/concepts/variables.mdx + docs/guides/color-grading.mdx now use clean-studio.

Bonus — my parity nit is also resolved, at compile-time and stronger than I suggested: the key types now derive from the contract arrays (type HfColorGradingAdjustKey = (typeof COLOR_GRADING_ADJUST_KEYS)[number]) and the runtime defaults are pinned with ADJUST_ZERO … satisfies Record<HfColorGradingAdjustKey, number> (same for details/effects). A key present in the array but missing from EFFECT_DEFAULTS (or vice-versa) now fails compilation — the single owner can no longer drift from the runtime defaults.

CI: no failing checks at 70213c5a.

Verdict: COMMENT — the blocker is resolved and this is ready on the merits; I'd APPROVE, but I'm holding the stamp for @jrusso1020's per-PR go (the author isn't on our trusted-stamper list). Given the word, I'll flip this to APPROVE.

Review by Jerrai (hyperframes)

@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.

Per @jrusso1020's go — flipping my verdict to APPROVE at 70213c5a. My round-2 blocker (removing four released preset IDs silently broke the media-use resolver) is resolved: the removal is completed in lockstep, deprecated phrases are remapped to surviving presets, a regression test pins the remap, and the docs are updated. My parity nit is closed at compile-time too (key types derive from the contract arrays; defaults are satisfies Record<…>-pinned). The core contract and the shared @hyperframes/parsers/color-grading-contract single-owner refactor are solid. Ready on my side.

Review by Jerrai (hyperframes)

@miguel-heygen miguel-heygen 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.

Exact-head re-review at 70213c5a8526b12c8b26f01b8288d78bb9edc917.

The prior contract concerns are resolved at the root: @hyperframes/parsers/color-grading-contract now owns the top-level, section, effect, and LUT registries; core exports typed views of that contract; and the shipped media-use resolver no longer emits the four removed preset IDs. The legacy natural/fresh/warm/cool phrases are deliberately remapped to surviving presets, with a regression test pinning each mapping and docs updated in lockstep.

No additive blocker found. Required checks are green; two regression shards were still running at review time with no failures.

Verdict: APPROVE
Reasoning: The current head establishes one schema owner and completes the preset removal across the released resolver, synonyms, tests, and docs without leaving a silent neutral-frame path.

— Magi

@ukimsanov
ukimsanov merged commit 9e7ddb0 into main Jul 24, 2026
82 of 92 checks passed
@ukimsanov
ukimsanov deleted the feat/media-treatment-schema branch July 24, 2026 16:58
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.

4 participants