Skip to content

docs(skills): teach speed ramps as a rate lane in one reference page - #4171

Closed
miguel-heygen wants to merge 2 commits into
mainfrom
feat/speed-ramp-docs
Closed

miguel-heygen wants to merge 2 commits into
mainfrom
feat/speed-ramp-docs

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What changes

Speed ramps are taught in one place. A new reference page, speed-ramps, explains the rate lane in data-automation: the lane shape, presets, what Studio shows, pitch handling, and limits.

Three lines that were wrong now that ramps exist are fixed: the data-playback-rate row in the HTML schema said 0.1 to 5 (the range is 0.1 to 10), and the audio, keyframes, main and editing-recipes skills said speed ramps are unsupported and to preprocess. The skills manifest is regenerated.

Verification

  • An independent fact-check opened the producer for every claim on the page (lane fields, clamp, log interpolation, curve meaning, lane over constant, presets) and found them accurate; two wording issues it raised (clip length needs data-duration; the render approximates the curve in quarter-second slices) are fixed here.
  • It also found that hyperframes snapshot still clamped the rate at 5 and ignored lanes; that is fixed separately in fix(cli): snapshot picks the frame through a rate lane and up to 10x #4170.
  • gen-skills-manifest reports in sync; docs.json is valid and lists the page once.
Audited Trusting Not exercised
every factual claim on the new page against core and engine source, the anchor, docs.json, remaining stale statements repo-wide the Studio wording (verified in the #4153 capture) a Mintlify preview build, a real render of the page's example

@mintlify

mintlify Bot commented Sep 19, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated
hyperframes 🟢 Ready View Preview Sep 19, 2026, 3:44 PM

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

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

COMMENT / HOLD at ce7d3151 — ordering dependency on unmerged #4153 + one stale recipe missed + a now-vacuous test assertion

Content is largely correct and well-organized — the new speed-ramps.mdx matches merged core/engine source (verified below). But three items should be addressed before this can safely auto-merge.

1. Ordering dependency (blocking stamp=MERGE) — #4153 is not merged

Your body says "merges after the Studio speed-ramp PR it describes." Confirmed merge state:

PR State Note
#4129 (speed-ramp core rate lane, MAX_PLAYBACK_RATE=10) MERGED 12:27 UTC
#4143 (renderer speed-ramp, atempo slicing) MERGED 14:01 UTC
#4153 (Studio Media Speed slider + A toggle + preset menu) OPEN
#4170 (CLI snapshot speed ramp + rate 10x) OPEN not required by this PR's text

The docs page's "Editing in Studio" section names the Speed slider, the A automation toggle, and the timeline rate lane — that UI ships in #4153. "Presets" names the Speed preset menu (Montage, Hero, Bullet, Jump cut, Flash in, Flash out) — also #4153. If #4171 auto-merges ahead of #4153, both sections describe UI that isn't there yet.

Holding the APPROVE for a live head after #4153 lands. If you'd prefer, I can APPROVE once #4153 shows state=MERGED; a nudge in this thread when it lands is enough.

2. Real doc miss — creator-editing-recipes.md:439 still contradicts the new rule

The PR body claim is "no page still says rates stop at 5x or that ramps are unsupported." One recipe still says both.

skills/hyperframes-core/references/creator-editing-recipes.md, in "Split a section and change its speed" (line 439 at ce7d3151):

Source math: <video> and <audio> parts use data-playback-rate (0.1 to 5, constant) per the constant-speed recipe above, with matching audio. Limit: a speed ramp (a rate that changes within one clip) is not in the format; approximate with several constant-rate parts or preprocess a derived asset through /media-use.

Both halves are wrong now: the range is 0.1 to 10, and a speed ramp is in the format as a rate lane. The other recipe in the same file (~line 227) was updated correctly; this one was missed. Small fix in the same PR unblocks the body's "repo-wide" framing.

3. Latent CI gap — assertions in scripts/creator-editing-capabilities.test.mjs no longer verify their stated intent

Two assertions still require the removed phrasing (line 99 and line 135):

assert.match(keyframes, /speed ramps?[\s\S]{0,300}(not supported|preprocess)/i);
assert.match(audio,    /speed ramps?[\s\S]{0,220}(not supported|preprocess)/i);

They pass on the new content only because [\s\S]{0,300} is wide enough to bridge across an intervening table row into the "mid-source freeze ... not supported; preprocess a still/derived segment" Freeze/hold row (keyframes SKILL.md), and similarly bridges to nearby /media-use "preprocessing" mentions (audio SKILL.md). Green today, but they no longer lock in what they were written to lock in — a future PR could re-introduce "speed ramps are not supported" verbatim and this suite wouldn't catch it. Consider inverting both to assert.doesNotMatch(...) for the old phrasing plus a positive assert.match(..., /rate lane/) for the new one — keeps the drift-lock while flipping the invariant.

Non-blocking on its own, but worth folding into the same push since you're already touching the skills.

Content verification — accurate against merged source

  • MAX_PLAYBACK_RATE = 10, MIN_PLAYBACK_RATE = 0.1 in packages/parsers/src/mediaDuration.ts at head; core playbackRateBounds.ts re-exports (verified in the #4166 clamp-unification arc). Docs "Clamped to 0.1 to 10" ✅.
  • sourceTimeAt(spec, t) in packages/core/src/speedRamp.ts at head, with the docstring "the one rate-to-time mapping consumers share" — matches your "one function, sourceTimeAt, which the preview and the renderer both use" ✅.
  • sampleAutomationLane(spec, t, "log") at head — matches your "logarithmic scale, so going from 0.5 to 2 passes through 1 at the midpoint" ✅.
  • t in clip seconds (shiftRateLane and rateAt both accept clip-local t; shiftRateLane reindexes for the trim case) — matches "Seconds from the start of the clip, not from the start of the composition" ✅.
  • Renderer atempo slicing — I verified RAMP_SLICE_SECONDS=0.25 and MAX_RAMP_SLICES=240 in my #4143 review at merged head 3f13e1e6; matches your "short constant-speed slices of a quarter second, which approximates the curve while keeping pitch" ✅.
  • Lane-wins-over-constant, data-duration fixes timeline hold, pitch preservation via browser default (HTMLMediaElement.preservesPitch=true, verified in my #4143 pitch-attr-removal review) — all consistent ✅.

Verified-clean negatives (peer-review style)

  • New page cross-links to /reference/audio-effects#the-four-attributes — the anchor is stable in existing docs (unchanged in this PR).
  • docs.json adds reference/speed-ramps once, sits under the same reference nav block as html-schema/audio-effects — nav shape sensible.
  • skills-manifest.json hashes updated for the four skills whose SKILL.md/references changed (hyperframes, hyperframes-audio, hyperframes-core, hyperframes-keyframes) — Skills manifest-in-sync CI check ✅.
  • Mintlify Deployment ✅ at head.

CI

Docs Validate docs still IN_PROGRESS at read-time; other required lanes SKIPPED per docs-only path filter (which also means Test (the broad suite) didn't run — that's how the test-file gap in §3 stayed green). Skills manifest + skills tests + format + lint + Mintlify all green at ce7d3151.

Stamp mechanics

hf-oss require_last_push_approval=true — I'll re-review at whichever head follows the #2/#3 fixes (or the current head if you push nothing and just wait for #4153 to merge; but this PR would then still need a re-APPROVE at that later head anyway per the setting).

— Review by tai (pr-review)

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

Re-review at 0eb7ea91 — HOLD stands, same reasons; recipe B in creator-editing-recipes.md still contradicts recipe A within the same file

Delta since my prior COMMENT at ce7d3151 (5 commits ahead / 2 behind; author-only commits: 55dfe335 "teach speed ramps as a rate lane in one reference page" + 0eb7ea91 "say what fixes a ramped clip's length and how the render approximates pitch"). The rebase pulled in #4165 (dock engine, merged), #4144 (docs, merged), #4169 (timeline, merged) — those aren't this PR's content.

Item 1 — Ordering dependency on #4153 (primary blocker, unchanged)

gh pr view 4153: still state=OPEN, no mergedAt. The docs page's "## Presets" (line 47) and "## Editing in Studio" (line 53) still name the Studio Speed preset menu, the A automation toggle, and the timeline rate lane — all UI that ships in #4153. Auto-merging this PR ahead of #4153 leaves those sections describing UI that isn't there yet.

Item 2 — Real doc miss: recipe B in creator-editing-recipes.md (line 491, was 439 at ce7d3151)

55dfe335 fixed recipe A at line 232 (constant-speed recipe): "Limit: normalized 0.1..10. For a speed ramp put a rate lane in data-automation, e.g. {...}; it wins over the constant."

Recipe B is unchanged. At 0eb7ea91 line 491, the "Split a section and change its speed" recipe still reads:

Source math: <video> and <audio> parts use data-playback-rate (0.1 to 5, constant) per the constant-speed recipe above, with matching audio. Limit: a speed ramp (a rate that changes within one clip) is not in the format; approximate with several constant-rate parts or preprocess a derived asset through /media-use.

Both halves still contradict the merged core (MAX_PLAYBACK_RATE=10 in packages/parsers/src/mediaDuration.ts) and the new rate lane in data-automation. And "0.1 to 5" here is not the viewer transport-speed range Miguel added at speed-ramps.mdx:62 — this recipe is about the authored data-playback-rate attribute, which is 0.1..10. It's the same finding as before, just at a shifted line number.

Same-file inconsistency between recipe A (correct) and recipe B (still wrong) is the specific tell. Small fix in the same PR would close this cleanly.

Item 3 — Test-regex bridging (non-blocking, unchanged)

scripts/creator-editing-capabilities.test.mjs:99,135 still uses /speed ramps?[\s\S]{0,300}(not supported|preprocess)/i and /speed ramps?[\s\S]{0,220}(not supported|preprocess)/i — coincidentally pass by bridging into a neighboring row (Freeze/hold's "mid-source freeze is not supported; preprocess a still/derived segment"). The new assert.doesNotMatch(keyframes, /keyframe(?:d|ing)?\s+(?:the\s+)?data-playback-rate/i) at line 101 is a positive whitelist for a different phrasing — welcome, but it doesn't replace the vacuous ramps assertion. Same-PR fold if you're already touching skills.

What's actually cleaner at this head

  • Recipe A at line 232 in creator-editing-recipes.md: correctly says 0.1..10 + rate-lane example ✅
  • skills/hyperframes-audio/SKILL.md: constant range 0.1..10 + "speed ramp is a rate lane in data-automation" ✅
  • speed-ramps.mdx: pitch-approximation quarter-second note, transport-speed cap explicit ✅
  • docs.json nav + skills-manifest hashes coherent

Stamp mechanics

reviewDecision: REVIEW_REQUIRED, mergeStateStatus: BLOCKED. hf-oss require_last_push_approval=true — will APPROVE at the head that (a) has recipe B fixed and (b) either #4153 merged, or the Studio-UI sections in speed-ramps.mdx qualified as "coming in a follow-up." Item 3 remains non-blocking.

— Review by tai (pr-review)

Adds the speed-ramps reference, corrects the 0.1 to 5 rate range to 0.1 to 10
and the 'speed ramps are not supported' lines in the skills, and regenerates
the skills manifest.

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

Re-review at bafe6422 — HOLD stands; both prior blockers unchanged (content-identical rebase after #4174 confirms it)

Miguel's re-pin flags this as a rebase after #4174 (skills manifest regenerated) with "content identical". Verified: content IS identical for the two things I was holding on.

Item 1 — Ordering dependency on #4153 (still open)

gh pr view 4153: state: OPEN, mergedAt: null, reviewDecision: REVIEW_REQUIRED, head b7aa702c. The docs page's "## Presets" and "## Editing in Studio" still name the Studio Speed preset menu, the A automation toggle, and the timeline rate lane — UI that ships in #4153. Merging this ahead leaves those sections describing UI that isn't there yet.

Item 2 — Recipe B in creator-editing-recipes.md (line 491)

Fetched the file at bafe6422; recipe B is byte-identical to 0eb7ea91. Line 491:

Source math: <video> and <audio> parts use data-playback-rate (0.1 to 5, constant) per the constant-speed recipe above, with matching audio. Limit: a speed ramp (a rate that changes within one clip) is not in the format; approximate with several constant-rate parts or preprocess a derived asset through /media-use.

Both halves still contradict merged core (MAX_PLAYBACK_RATE=10 in packages/parsers/src/mediaDuration.ts) and the new rate lane in data-automation — which recipe A at line 232 in the same file describes correctly (Limit: normalized 0.1..10. For a speed ramp put a "rate" lane in "data-automation"…). Same-file A-vs-B inconsistency stands.

"0.1 to 5" here is not the viewer transport-speed range Miguel added at speed-ramps.mdx:62 — recipe B is about the authored data-playback-rate attribute, which is 0.1..10. Same finding as ce7d3151 and 0eb7ea91; the rebase moved nothing about it.

What will lift HOLD

  • Either #4153 merged (Somu owns it at b7aa702c), OR the Studio-UI sections in speed-ramps.mdx re-scoped as "coming in a follow-up." AND
  • Recipe B updated to match recipe A (0.1..10 + rate-lane pointer).

Item 3 (test regex bridging at creator-editing-capabilities.test.mjs:99,135) remains non-blocking.

— Review by tai (pr-review)

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Folded into #4153 (one speed-ramp feature, one PR).

This branch was successfully deployed

1 active deployment
staging - docs bafe6422 Deployed Sep 19, 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.

2 participants