feat(cli): add agent-first media treatment workflow#2753
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an agent-first “media treatment” workflow to HyperFrames by introducing a canonical hyperframes media-treatment CLI surface for capability discovery + deterministic persistence, and by expanding the /media-use skill docs/tests to route broad “footage looks…” feedback into that workflow (including new offline cached error-diffusion tooling).
Changes:
- Introduces
hyperframes media-treatmentCLI command (capability overview/detail + apply/clear on a single<img>/<video>viadata-color-grading). - Updates
/media-useguidance, recipes, and tests to treat broad visual feedback as media-treatment intent (plus addsresolve --analyzefor side-effect-free grade evidence). - Adds exact cached error-diffusion dithering tooling + tests, and expands LUT/preset intent routing synonyms.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/media-use/SKILL.md | Expands routing guidance to media treatments and progressive capability discovery. |
| skills/media-use/scripts/resolve.test.mjs | Adds coverage for --analyze help + side-effect-free grade evidence behavior. |
| skills/media-use/scripts/resolve.mjs | Adds --analyze mode for grade evidence without recording a manifest candidate. |
| skills/media-use/scripts/lib/lut-preset-provider.test.mjs | Adds deterministic intent-alias tests for new/curated preset routing. |
| skills/media-use/scripts/lib/lut-preset-provider.mjs | Expands preset IDs/synonyms; keeps legacy IDs readable while de-emphasizing them in fuzzy matching. |
| skills/media-use/scripts/lib/error-diffusion.test.mjs | Adds golden tests for deterministic error-diffusion kernels + contract validation. |
| skills/media-use/scripts/lib/error-diffusion.mjs | Implements error-diffusion kernels and public apply function for RGBA buffers. |
| skills/media-use/scripts/dither.test.mjs | Adds end-to-end tests for dithering images/videos (palette correctness, audio/metadata, HDR rejection). |
| skills/media-use/scripts/dither.mjs | Adds offline cached error-diffusion dither CLI (image/MP4) built on FFmpeg + error-diffusion lib. |
| skills/media-use/references/operations.md | Documents “Exact error-diffusion dither” workflow and when to use it. |
| skills/media-use/references/media-treatments.md | Adds media-treatment policy: lanes, verification workflow, capability discovery, overlay composition rules. |
| skills/media-use/references/media-treatment-recipes.md | Adds a comprehensive set of tested seed recipes for common treatment intents. |
| skills/media-use/references/grading.md | Reframes “grade” usage around the media-treatment workflow and deterministic persistence/verification. |
| skills/hyperframes/SKILL.md | Routes broad photographic-media feedback to /media-use and its media-treatment policy. |
| skills/hyperframes/references/capability-menu.md | Updates capability menu row to include “media treatments” language and routing. |
| skills/hyperframes-cli/references/compare-and-batch.md | Updates grading JSON example to match the canonical nested adjust shape. |
| skills/general-video/SKILL.md | Updates cross-cutting adapter guidance to route vague footage feedback via media-treatments policy. |
| skills-manifest.json | Refreshes skill hashes/file counts after content updates. |
| packages/cli/src/templates/_shared/CLAUDE.md | Adds a new-project instruction gate for media-treatment policy before editing real media looks/reveals. |
| packages/cli/src/templates/_shared/AGENTS.md | Mirrors the new-project instruction gate for media-treatment policy. |
| packages/cli/src/help.ts | Adds media-treatment to CLI help groups. |
| packages/cli/src/commands/coreSkillContent.test.ts | Adds tests to ensure routing/docs/recipes stay synchronized (incl. palette sync with Core). |
| packages/cli/src/commands/color-grading.ts | Implements media-treatment command: capability discovery + deterministic HTML mutation/clear. |
| packages/cli/src/commands/color-grading.test.ts | Adds unit tests for capability discovery and deterministic application/clearing behavior. |
| packages/cli/src/cli.ts | Registers the media-treatment command loader. |
| packages/cli/src/cli.commands.test.ts | Asserts media-treatment is registered and color-grading is not. |
| packages/cli/src/capture/agentPromptGenerator.ts | Adds capture prompt guidance to use /media-use media-treatment policy for later media edits. |
| docs/guides/color-grading.mdx | Expands docs to include effect families/overlays, richer schema examples, performance notes, and CLI guidance. |
| docs/concepts/variables.mdx | Updates example preset enum defaults to the curated clean-studio preset. |
💡 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.
Audited: packages/cli/src/commands/color-grading.ts and tests; skills/media-use/scripts/dither.mjs, lib/error-diffusion.mjs, their tests, the resolve --analyze delta, LUT preset parity, command registration, and CI at exact head d91c2f4b.
Trusting: the 991-line recipe catalog was sampled against its positive-pin tests rather than recipe-reviewed line by line; generated skill-manifest hashes and documentation-only wording outside the new workflow were not independently regenerated.
The canonical core registries are used at the CLI boundary, persisted grading is normalized/serialized deterministically, and the standalone preset mirror has an exhaustive source-parity test. Copilot already covers the stale alias sentence and success-message wording; I did not repeat those.
Important: skills/media-use/scripts/lib/error-diffusion.mjs:131-136 and skills/media-use/scripts/dither.mjs:197-200 allocate three Float32 error slots for every source pixel even though default pointSize=3 reads/writes error only at one block center. At 4K this is ~100 MB for the error grid alone (~133 MB with the RGBA frame), while a block-grid buffer is ~11 MB. Index by block coordinates, or retain only the kernel row window, before this becomes the default agent path for high-resolution footage.
Important: skills/media-use/scripts/dither.test.mjs:126-145 asks ffprobe for frame=pts_time. On FFmpeg 4.2.7 the two keyframes are present but that field is omitted, so the test parses blank output as [NaN]; pkt_pts_time and best_effort_timestamp_time both report 0, 1. Use the portable timestamp field so the regression guard works across supported developer FFmpeg versions. Local focused result: 17 tests passed, this one assertion failed; the exact-head GitHub checks are green.
Ponytail: packages/cli/src/commands/color-grading.ts:558-574: yagni: reportMutation and runMutation are nested one-call wrappers whose only caller is the final branch. Inline the reporting block after prepareMutation() and remove the ReturnType plumbing. net: -12 lines possible.
— Magi
Verdict: APPROVE
Reasoning: No correctness or persistence-contract blocker found; the two important items are bounded performance/test-portability follow-ups, and exact-head CI is green.
jrusso1020
left a comment
There was a problem hiding this comment.
Additive review at d91c2f4b, weighted toward Ular's focus (duplication / deletable / over-complex / core contract-match). @Copilot commented (the media-treatment success message still says "color grading"; docs claim a color-grading alias the tests say isn't registered). Not repeating those; per Ular, 1/3 issues are being fixed and I'm not re-reporting them.
Good — the CLI is a real core consumer, not a re-implementation. applyColorGradingToHtml routes through core's normalizeHfColorGrading → isHfColorGradingActive → serializeHfColorGrading and reuses patchElementInHtml for the mutation; the capability views are a presentation layer over getHfColorGradingCapabilities(). adjust/details/effects validation sets derive from the core registries (color-grading.ts:331-333). The --file path guard (isPathInside + .html, color-grading.ts:518) is correct.
Additive (duplication) — the same hard-coded-key pattern Magi blocked on #2752's lint rule recurs here, and it's rooted in a #2751 gap. GRADING_KEYS (color-grading.ts:320) and LUT_KEYS (:334) are hand-copied literals of the top-level HfColorGrading shape and HfColorGradingLutRef, right next to the three sibling sets that do derive from core. Core exports HF_COLOR_GRADING_{ADJUST,DETAIL,EFFECT}_KEYS but no top-level or lut registry, so every consumer re-hardcodes those two — the lint rule (media.ts, Magi's blocker) and now this CLI. They match today, but the next valid top-level/lut control silently becomes a hard CLI error (assertKnownGradingShape throws) with green CI. The clean fix is one #2751-owned HF_COLOR_GRADING_TOP_LEVEL_KEYS + HF_COLOR_GRADING_LUT_KEYS that both the lint rule and this CLI import — i.e. the #2752 fix should extend here rather than land twice. (Inline below.)
Same family, weaker: EFFECT_DISCOVERY_ROWS (:29-48) is satisfies-checked for membership (every key is a valid active effect) but not for completeness — a new core active effect wouldn't error, it'd just be silently absent from media-treatment --capabilities. An exhaustiveness assertion against HF_COLOR_GRADING_ACTIVE_EFFECT_KEYS would close it.
Verified — no runtime duplication in the offline dither tooling (a duplication Ular explicitly asked about). skills/media-use/scripts/lib/error-diffusion.mjs is CPU error-diffusion (Floyd–Steinberg / Atkinson / Stucki / …) for baking static assets; the runtime's dither effect (#2752) is a GPU ordered dither for live frames. Different algorithm class, execution context, and output — complementary, not overlapping. Its palette validation (2-6 × #rrggbb) matches core, and re-stating it there is correct since skill scripts are intentionally standalone (no workspace-build import). (@magi separately flagged a ~100 MB error-buffer allocation in the default dither path — a perf issue orthogonal to this duplication question.)
Verdict: COMMENT — Ready on the merits once the top-level/lut key source is unified with the #2752 fix (latent today, not blocking on its own). Clean, well-scoped consumer of the core contract. (Deferring the approving stamp to the maintainer.)
Review by Jerrai (hyperframes)
d91c2f4 to
a96419f
Compare
f9bf4d0 to
f0ee0db
Compare
a96419f to
d0d9c38
Compare
f0ee0db to
7a911c3
Compare
d0d9c38 to
ba10d66
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review at ba10d666. @magi is re-reviewing batch 2 in parallel; posting my independent verdict.
My prior finding is resolved. The CLI command (color-grading.ts → renamed media-treatment.ts) no longer hard-codes the top-level / LUT key lists — the GRADING_KEYS / LUT_KEYS literals are gone. It now imports validateColorGradingContract from @hyperframes/parsers/color-grading-contract (used at :250) and consumes core's normalizeHfColorGrading / serializeHfColorGrading / getHfColorGradingCapabilities. So the CLI is now the third consumer of the single shared contract — the "three consumers each re-hardcode the schema" pattern across this stack (lint #2752, CLI #2753, and core) is fully closed. New media-treatment.test.ts (+271) covers the command. No failing CI at ba10d666.
Verdict: COMMENT — ready on the merits; my prior finding is resolved and nothing new surfaced. 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.
Audited: the delta from my prior review through exact head ba10d666; the dither error-buffer/indexing change and focused image/video tests; FFprobe timestamp portability; the CLI rename/consolidation; shared parser-owned contract validation; current reviews/threads; and exact-head check runs.
Trusting: the large stacked #2751/#2752 core/runtime delta was reviewed separately at its own exact heads; recipe/documentation prose was sampled rather than line-reviewed; regression shards still running at submission are left to the branch gate.
My two prior important findings are resolved. Error diffusion now allocates/indexes a block grid (ceil(width / pointSize) × ceil(height / pointSize) × 3) instead of a per-source-pixel grid, including reuse across video frames. The keyframe regression uses portable best_effort_timestamp_time; the focused dither suite passes locally (7/7, including MP4/audio/BT.709). The CLI now delegates contract validation to the shared parser contract, consumes core capability discovery, and the nested one-call reporting wrappers are gone.
No new correctness, persistence, duplication, or unnecessary-complexity blocker found. Exact-head preflight, preview, and performance checks are green; eight regression shards are still in progress with no failures at review time.
— Magi
Verdict: APPROVE
Reasoning: The prior performance and portability findings are fixed with regression coverage, the schema ownership is consolidated, and the current delta is sound; remaining in-progress shards are a branch gate rather than a review finding.
7a911c3 to
944640c
Compare
ba10d66 to
e65276f
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
Freshness re-review at exact head e65276f1c.
Audited: patch identity versus my prior approved head ba10d666 (range-diff is exactly =); current contract-validation/capability call sites; the prior dither allocation and FFprobe portability fixes; every review thread; and all current-head required and extended checks.
Trusting: the large recipe/documentation catalog remains sampled rather than line-reviewed; the stacked #2751/#2752 runtime surface was reviewed at its own exact heads.
The substantive patch is byte-equivalent after the rebase. The CLI still delegates validation to the shared parser contract and normalization/serialization to core (packages/cli/src/commands/media-treatment.ts:250, :299-301), with contract exhaustiveness pinned in packages/cli/src/commands/media-treatment.test.ts:43. The dither fix still allocates and indexes by block grid (skills/media-use/scripts/lib/error-diffusion.mjs:114, :135-189) and reuses that bounded buffer across video frames (skills/media-use/scripts/dither.mjs:197-198, :287). The portable keyframe timestamp field remains pinned (skills/media-use/scripts/dither.test.mjs:135).
All three review threads are resolved/outdated. Exact-head required checks are green, as are all eight regression shards, Windows render/tests, CodeQL, CLI npx shims, global install, skill lint/manifest, and preview/perf lanes.
— Magi
Verdict: APPROVE
Reasoning: The rebase preserves the previously approved patch exactly, the prior performance/portability/SSOT findings remain resolved, and exact-head CI is fully green.
jrusso1020
left a comment
There was a problem hiding this comment.
APPROVE at e65276f1 per @jrusso1020's go. Re-verified after the #2752 merge: this is a clean rebase — #2753's own content is byte-identical to the patch I reviewed at ba10d666 (the packages/cli/skills delta since then is empty), my earlier finding stays resolved (the CLI media-treatment command consumes the shared @hyperframes/parsers/color-grading-contract validator rather than re-hardcoding keys), and the full required suite is green at this head. Matches @magi's byte-equivalence confirmation.
Review by Jerrai (hyperframes)
What
Adds the canonical
hyperframes media-treatmentcommand, progressive capability/detail output, deterministic apply/clear operations, media-use guidance and recipes, public docs, and offline error-diffusion tooling.Why
Agents should discover a small capability menu, inspect only the relevant treatment, assemble bounded custom payloads, apply them deterministically, and verify results without learning the low-level HTML attribute first.
How
Routes look, effect, animation, and overlay intent through one CLI surface. Skills teach agents to use real media elements, combine compatible operators, preserve user intent, offer unsolicited polish once, and verify representative frames.
Test plan
Stack 3/6. Base: #2752. Registry overlays and Studio UI are optional later layers.