Skip to content

docs: make the pages show the thing they describe - #3010

Merged
ukimsanov merged 71 commits into
mainfrom
docs/pages-show-not-tell
Aug 5, 2026
Merged

docs: make the pages show the thing they describe#3010
ukimsanov merged 71 commits into
mainfrom
docs/pages-show-not-tell

Conversation

@ukimsanov

@ukimsanov ukimsanov commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Sits on top of #2976, so it reads as the sixth step of stack #2980. Everything here answers one complaint: pages describe a capability instead of showing it, and where they do explain, they explain with spreadsheets.

What was measured first

307 pages, 177,268 words, of which 136 are hand-written — 169 catalog pages and the changelog are generated, so fixing a script beats fixing pages.

65 pages showed the reader nothing at all. 15 of those were guides, which are "how do I do X" pages.

The catalog generator had two real bugs, not style problems

It deletes hand-written documentation on every run. rmSync(dir, { recursive: true }) on both catalog folders before regenerating. Twenty-five pages carried sections that exist in no registry-item.json. There is now a carry-forward pass, verified stable across three consecutive runs. It prevents future loss only — the regeneration in #2975 already removed what was there, and it is recoverable from git if anyone wants it back.

It was breaking the sidebar. Rebuilding the Catalog tab dropped its icon and unlinked catalog/index.mdx entirely.

And 27 of 36 component pages told the reader to "see the comment header in the file" when no such header exists. The generator reads the file now, so 7 pages say it and all 7 are true.

Presentation: 1,365 table rows across 168 pages became 0. ## Details was a table whose rows were "Type: Block" and a duration; ## Files was three columns where most items have exactly one file. The preview leads the page now instead of sitting under title, description, tags, a warning and a credit — four of which Mintlify already renders from frontmatter.

Four pages rewritten

Color grading was the worst page in the docs by the measure that matters: 33 table rows against one image, for the most visual feature in the product. Four of five tables are prose now. Four factual errors were found and verified at source: the Studio panel is Grade in the flat Inspector, the controls are whites/blacks not "white point/black point", LOG and HDR were conflated, and project-wide LUT copy refuses a relative path in favour of a URL or data URL. It also documents two things it never mentioned — grade-compare, and that data-color-grading must nest under adjust/details/effects or it renders nothing at all.

vs Remotion argued for 851 words with nothing to look at and no code. It now carries the same three-second title card written both ways. The HyperFrames one is a real composition, not illustrative markup — extracted and run through lint (0/0) and the full browser gate, with runtime, layout, motion and contrast all passing. It concedes what Remotion genuinely does better.

Choose how to create never said what the four options were before comparing them. Bring in a design loses a 7-row table for one plain rule, and a bullet list that used semicolons.

Four guides now show the thing

Media effects named eighteen effects as a bulleted list of words. Background removal and HDR showed nothing at all. GSAP animation — a page about motion — showed nothing.

The HDR one is the honest case: you cannot show HDR brightness on an SDR page, so it does not try. It shows the ffprobe verdict on a real render and a measured round-trip, 4000 nits authored against 4012 back out, and says on screen for all 26 seconds that nothing is simulated.

Every film was checked before publishing: no black or frozen stretches, and read at the real 590px docs column width rather than full size.

Examples loses the ten-second reference-project render. It sat directly beneath four finished films and lost the comparison every time; the cards above already carry the point.

Not done, deliberately

  • Catalog pages still do not show source. shadcn does because you paste 40-80 lines; our blocks have a median of 285 and install with one command. Different distribution model.
  • The changelog is 27,000 words on one page. Splitting it breaks 7 inbound links and the RSS feed — a product call, not a docs one.

Note for review: docs/packages/cli.mdx

This page was rewritten (963 insertions, 1002 deletions) and the change is
committed under 59230124 fix(scripts): drop the catalog poster…, which is
not what that commit message describes. I ran git add -A docs/ while an
unrelated rewrite was sitting uncommitted in the working tree. The content is
fine; the history is misleading, and I would rather say so than let a reviewer
find a thousand-line page rewrite hiding under a commit about poster images.

What changed on that page:

  • 9,600 → 8,875 words, 299 → 270 table rows, 83 → 68 headings.
  • A stale examples table is gone. It listed five --example values when
    there are nine, so it was already wrong. It now links to /examples, which is
    the canonical list, instead of keeping a second copy that drifts.
  • --max-screenshots no longer documents behaviour that does not exist. The
    page claimed a default of 24; capture/index.ts:63 destructures it as
    _maxScreenshots — underscore-prefixed, unread. It now says the flag is
    accepted and has no effect.

Every anchor that another page links to survived — #render, #check, #init,
#skills, #telemetry, #media-treatment, #remove-background,
#hyperframes-auth, #hyperframes-cloud, #hyperframes-lambda. #hyperframes-json
was dropped; nothing links to it.

Copilot AI review requested due to automatic review settings August 4, 2026 09:32

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

Updates the docs and changelog-drafting scripts so pages “show the thing” (video-first examples and clearer install/paste steps), while also tightening the changelog drafting/release gates with stronger human-writing placeholders and bullet formatting.

Changes:

  • Add a “style bar” marker alongside the release-summary TODO in generated changelogs and ensure release gates fail unless both markers are removed.
  • Terminate generated changelog bullets with periods (including weekly digests) and update tests accordingly.
  • Expand multiple guides and Catalog pages to lead with real video previews and replace repetitive tables with direct install/paste instructions.

Reviewed changes

Copilot reviewed 187 out of 187 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
scripts/set-version.ts Adds CHANGELOG_STYLE_NOTE marker and updates TODO detection to include it for release gating.
scripts/draft-changelog.ts Stamps the TODO + style-bar block into generated release notes/docs updates; terminates bullets with periods.
scripts/draft-changelog.test.ts Updates expectations for bullet punctuation.
scripts/changelog-weekly.ts Stamps weekly TODO + style-bar block; terminates weekly bullets with periods; expands publishing checklist.
scripts/changelog-weekly.test.ts Adds coverage for bullet termination and style-bar stamping.
docs/guides/remove-background.mdx Adds embedded video example via DocsVideo.
docs/guides/product-launch-video.mdx Updates lead example caption/media URLs for the workflow showcase.
docs/guides/hdr.mdx Adds embedded video and clarifies SDR/HDR verification framing.
docs/guides/gsap-animation.mdx Adds embedded video example for keyframes/ease shaping.
docs/examples.mdx Reworks the “reference project” section into brief/source/revision/checks cards and a concrete prompt excerpt.
docs/concepts/frame-adapters.mdx Adds an explanatory mermaid sequence diagram for deterministic frame seeking.
docs/catalog/components/yt-screen-warp.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/yt-feather-highlight.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/yt-circle-pointer.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/yt-camera-move.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/vignette.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/shimmer-sweep.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/parallax-zoom.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/parallax-unzoom.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/motion-blur.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/morph-text.mdx Adds poster attribute + install + paste instructions; moves tags to generated footer.
docs/catalog/components/mk-usage-arc.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/mk-emphasis-type.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/hw-underline.mdx Adds install + paste instructions and notes extra font asset; moves tags to generated footer.
docs/catalog/components/hw-callout-circle.mdx Adds install + paste instructions and notes extra font asset; moves tags to generated footer.
docs/catalog/components/hw-box-label.mdx Adds install + paste instructions and notes extra font asset; moves tags to generated footer.
docs/catalog/components/hw-boil.mdx Adds install + paste instructions and notes extra font asset; moves tags to generated footer.
docs/catalog/components/hw-arrow.mdx Adds install + paste instructions and notes extra font asset; moves tags to generated footer.
docs/catalog/components/grid-pixelate-wipe.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/grain-overlay.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-weight-shift.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-pill-karaoke.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-particle-burst.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-parallax-layers.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-neon-glow.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-neon-accent.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-matrix-decode.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-kinetic-slam.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-highlight.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-gradient-fill.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-glitch-rgb.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-emoji-pop.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-editorial-emphasis.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-clip-wipe.mdx Adds poster + install + paste instructions; moves tags to generated footer.
docs/catalog/components/caption-blend-difference.mdx Adds install + paste instructions; moves tags to generated footer.
docs/catalog/blocks/yt-vertical-fill.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/yt-prism-title.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/yt-lower-third.mdx Adds install + embed snippet (data-* attrs) instructions and notes extra asset; moves tags to generated footer.
docs/catalog/blocks/yt-logo-intro.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/yt-lcd-background.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/yt-comment-card.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/x-post.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/world-map.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/whip-pan.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/us-map.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/us-map-hex.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/us-map-flow.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/us-map-bubble.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/ui-3d-reveal.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-scale.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-radial.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-push.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-other.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-mechanical.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-light.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-grid.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-distortion.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-dissolve.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-destruction.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-cover.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-blur.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/transitions-3d.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/tiktok-follow.mdx Adds install + embed snippet (data-* attrs) instructions and notes extra asset; moves tags to generated footer.
docs/catalog/blocks/thermal-distortion.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/swirl-vortex.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/spotify-card.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/spain-map.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/sdf-iris.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/ripple-waves.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/ridged-burn.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/reddit-post.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/news-ticker.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-specs-list.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-progress-stat.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-placeholder-grid.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-line-graph.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-callout-highlight.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/mk-background.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/macos-notification.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-stack-bars.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-soft-pill.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-side-rule.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-kicker-name.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-dark-card.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-color-block.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-clean-bar.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/lt-bold-block.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/light-leak.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/instagram-follow.mdx Adds install + embed snippet (data-* attrs) instructions and notes extra asset; moves tags to generated footer.
docs/catalog/blocks/gravitational-lens.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/glitch.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/flowchart.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/flowchart-vertical.mdx Adds install + embed snippet (data-* attrs) instructions for portrait dimensions; moves tags to generated footer.
docs/catalog/blocks/flash-through-white.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/domain-warp-dissolve.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/cross-warp-morph.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-typing.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-snippet-flight.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-scroll.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-morph.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-highlight.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/code-diff.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/cinematic-zoom.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/chromatic-radial-split.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.
docs/catalog/blocks/app-showcase.mdx Adds install + embed snippet (data-* attrs) instructions; moves tags to generated footer.

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

Comment on lines +26 to +29
const WEEKLY_REVIEW_TODO = [
"<!-- TODO: review and rewrite before publishing. -->",
"<!-- Style bar: keep sentences under 25 words. Use everyday words. Avoid semicolons. Say what changed for the user, then why it matters. -->",
].join("\n");
@ukimsanov
ukimsanov force-pushed the docs/quality-system branch from 75df201 to da35679 Compare August 4, 2026 10:07
@ukimsanov
ukimsanov force-pushed the docs/pages-show-not-tell branch from ad308af to 9b72fd8 Compare August 4, 2026 10:07
Three pages that described capability instead of showing it.

**Media effects** named eighteen effects as a bulleted list of words and showed
four of them in one small PNG. A reader could not see what Kuwahara Paint,
Ordered Dither or CRT Curvature actually look like. It now leads with the
finished media-treatments showcase film — every effect family on real footage,
with labelled before/afters — and the "Choose the effect" table is gone, because
six rows were saying what four sentences say better.

**Product launch** embedded a 44-second cut of small UI panels on black. Replaced
with the film this workflow's own skill produced from huly.io: the product's real
screens, its type and colour, bold editorial layout. Verified before publishing —
no black stretches over 0.4s, no freezes over 1s, captions clean at full
resolution.

**Examples** gets its four project cards back. Cutting them to one link was my
misreading of the actual complaint, which was about the reference-project video
sitting underneath four finished films and losing to them.

Both new assets are published to the docs CDN under versioned filenames and
return 200.
**Color grading** was the worst page in the docs by the measure that matters:
33 table rows against one image, for the most visual feature in the product.
Four of the five tables were restating jargon in a grid — "Waveform | Brightness
by horizontal image position" tells you nothing you could act on. Those are prose
now, and the scopes section explains what each one is *for* (skin tones land on
one known line of a vectorscope, so faces are quick to check). The control lookup
stays a table, because that one is a genuine lookup.

Four factual errors in the old page, each verified at source before changing:
Studio's group is **Grade** in the flat Inspector, not "Color grading" in Design;
the controls are `whites`/`blacks`, not "white point/black point"; LOG and HDR
were conflated into one sentence when they are different cases; and the
project-wide LUT copy refuses a relative path in favour of a URL or data URL, not
a "project-root path".

Two things the page never mentioned and should have: `hyperframes grade-compare`,
which renders every candidate look onto one reference frame, and the fact that
`data-color-grading` nests under adjust/details/effects — a flat object renders
nothing at all.

**vs Remotion** argued for 851 words with nothing to look at and no code. It now
carries the same three-second title card written both ways. The HyperFrames one
is a real composition, not illustrative markup: extracted and run through lint
(0/0) and the full browser gate — runtime, layout, motion and contrast all pass.
It also concedes what Remotion genuinely does better, because a comparison page
that only flatters us is not worth reading.

**Choose how to create** never said what the four options were before comparing
them. It opens with the actual decision now — they all give you a video, the
difference is what you keep — and a four-row table of what you walk away with.

**Bring in a design** loses the 7-row keep-vs-rebuild table for one plain rule,
and the bullet list that used semicolons.

Every internal link on all four checked against the real page set. Where a visual
belongs but no approved asset exists, there is a JSX comment marking the spot
rather than a URL that would 404.
… and HDR do

**The catalog generator destroys hand-written documentation on every run.** It
does `rmSync(dir, { recursive: true })` on both catalog folders before
regenerating, so any section a human added to a generated page is deleted the
next time anyone runs it. There is now a carry-forward pass that reads those
sections off the existing pages first and re-emits any heading the template does
not own. Note this prevents future loss only — the earlier regeneration in this
stack already removed what was there, and it is recoverable from git if wanted.

It was also clobbering `docs.json`: rebuilding the Catalog tab dropped its icon
and unlinked `catalog/index.mdx` from the sidebar entirely.

And 27 of 36 component pages told the reader to "see the comment header in the
file" when no such header exists. The generator reads the file now and only says
it when true.

On presentation: 1,365 table rows across 168 pages became 0. `## Details` was a
table whose rows were "Type: Block" and the duration; `## Files` was three
columns where most items have exactly one file. Both are one sentence now. The
preview leads the page instead of sitting under title, description, tags, a
warning and a credit — four of which Mintlify already renders from frontmatter.

**Two guides that showed nothing now show the thing.** Background removal plays a
real cutout with the matte magnified at the hair edge and at a shoulder a plant
overlapped, because those are where matting fails. HDR is the honest one: you
cannot show HDR brightness on an SDR page, so it shows the `ffprobe` verdict and
a measured round-trip — 4000 nits authored, 4012 back out — and says on screen
for all 26 seconds that it is not simulating anything.

Both films were checked before publishing: no black or frozen stretches, and read
at the real 590px docs column width rather than full size. Nav after regeneration:
307 pages, 0 dangling, 0 redirect collisions.
The animation guide had nothing to look at, which is a strange state for a page
about motion. It now opens with the keyframes launch film — real `gsap.to()`
source with a `keyframes` array on screen, and the same motion being reshaped by
dragging an ease curve in Studio instead of guessing numbers.

Examples loses the ten-second reference-project render. It sat directly beneath
four finished films and lost the comparison every time; the four cards above it
already carry the point that the project is open end to end. The cards stay, the
render goes.
I stripped the audio track with -an when encoding for the web. The source has a
real mix at -19.2 dB mean / -3.1 dB peak, and the docs player is click-to-play,
so there is no reason for it to be silent. Re-encoded with AAC at 128k; levels
match the source exactly.

The other published films have no audio track at all to preserve — they were
rendered silent at source.
Ten videos on the docs site played in silence. The player is click-to-play, so a
reader deliberately starts them and gets nothing — which reads as broken rather
than as a stylistic choice.

The music is from HeyGen's own catalogue, pulled through the media-use skill's
resolve path with the account already authenticated on this machine. That matters
more than convenience: the launch projects on disk carry a `launch-music-trimmed.mp3`
with no licence, no attribution and no metadata, and three Pixabay tracks. Neither
belongs on the official docs CDN, where objects cache immutably for a year.

Mixed at roughly -22 to -24 dB mean so it sits under the picture. Getting there
took two corrections worth recording: proportional fades, because a 1.2s fade-out
on a 5s clip eats half the audio; and seeking 11s into the bed, because its first
ten seconds are a quiet intro that dragged every short clip down to -27 dB.

The three Examples tiles are left silent on purpose — they are muted autoplay
loops, so an audio track would only be weight.

Verified: every video referenced anywhere under docs/ that is not a muted
autoplay tile now has an AAC stream, checked against the live CDN URL rather than
the local file.
Both went to /guides/color-grading anchors that the guide rewrite removed. The
content did not disappear — it lives on /reference/color-grading as 'Animate a
supported property' and 'Limit a grade to part of the frame'. A reader clicking
either landed on the right page and scrolled nowhere.

mint broken-links does not check anchors, so CI would never have caught this.
HTML in Canvas, MCP, Skills, and Images and video all had nothing to look at.

**HTML in Canvas** puts a real HTML dashboard beside the same pixels running as a
live WebGL texture, bent by a shader while the DOM underneath keeps updating and
the texture re-captures every frame. That is the whole feature in one shot.

**MCP** is an honest reconstruction and says so on screen for its full length —
"Illustration of the flow". It could not reach a live connector, so it does not
pretend to have one. Describe the video, revise it in plain language, ask for a
format, get the render.

**Skills** shows a real run of the install command, then the router reading two
different requests and picking a different workflow for each.

**Images and video** walks the five basic moves — import, place, crop, trim, swap
the source — adding one attribute at a time on the left with the result on the
right. It serves both /guides/media and /guides/video-components.

All four carry a music bed from HeyGen's own catalogue at about -22 dB. Each was
checked on a contact sheet before publishing, not taken on the session's word.

Blind guides: 15 down to 9.
…ore concepts

**Prompting.** These were the only two pages that stayed hard to read once the
sentence counter was corrected for bullet lists. Capstone went from 46% of
sentences over 28 words to 20%, and 53 semicolons to 42. Motion went from 37% to
15%, 26 semicolons to 4, and lost six table rows that were prose in a grid. Both
keep every technique and every worked example — the length barely moved, the
sentences did.

**Concepts.** Compositions and Determinism explain how the whole thing works and
showed the reader nothing. Each has one mermaid diagram now: how an HTML file
with data-* timing becomes a numbered sequence of frames, and why frame 90 always
comes out the same — what is locked before frame 0, and the three things that
break it.

Mermaid renders natively in Mintlify, so these cost no asset, no CDN upload and no
regeneration. Verified in the browser rather than assumed; the pattern is the one
proven on concepts/frame-adapters.
@ukimsanov
ukimsanov force-pushed the docs/pages-show-not-tell branch from 9b72fd8 to eb199bd Compare August 4, 2026 10:08
The capstone and determinism rewrites renamed headings that other pages point at.
Eighteen prompting pages linked to capstone#the-full-prompt-verbatim, which is now
'The prompt, word for word', and the rewritten determinism page invented an anchor
on frame-adapters that was never there.

Moved the links rather than restoring the old headings — 'the full prompt,
verbatim' is exactly the register the rewrite was cleaning up.

Broken anchors site-wide: 0. mint broken-links does not check anchors, so none of
this would have failed CI.
Rendering, Performance, Deploy, Figma and Bring in a design.

**Figma is the one worth reading about.** The first attempt failed honestly — no
token, no file key, so it refused to fake an import and built a generic design
handoff instead, where both panels looked identical and nothing was demonstrated.
A real token and file key were sitting in bug-bash-figma-hf. With genuine access
it makes the actual point: the same cursor and the same clicks land on a PNG export
and on the imported HTML, and only the HTML responds — the field takes text, the
button runs through its states. One side is pixels, the other is a component. It
replaces the placeholder comment on Bring in a design too.

**Performance is measured, not asserted.** 25.0s against 9.8s as the median of
three runs at 1920x1080 over 300 frames, one CSS declaration responsible for the
whole gap, and a pixel comparison at 43.7 dB PSNR confirming the picture did not
change. Real numbers from real runs on this machine.

**Rendering** shows the loop end to end. **Deploy** shows the four places a render
can run and labels anything not executed live.

All four carry their own audio at about -19 dB. Each was checked on a contact
sheet and for dead frames before publishing.

Guides with nothing to look at: 15 at the start of this work, 5 now. The five
left are pages where a video would be decoration — authentication, feedback, a
decision table, a comparison that carries code, and 4K, which is a paragraph
about a flag.
The page that explains how the whole thing works had one Studio screenshot, 13
table rows, and two ASCII trees doing a diagram's job.

Two mermaid diagrams now. The first is the page's central claim made visible —
agent, Studio, CLI, SDK and Player all reading and writing one project folder,
with the render falling out of it. The second replaces the ASCII composition
tree with the same structure carrying real times and the file each scene lives
in, which the ASCII version could not show.

Verified rendering in the browser, not assumed.
Base automatically changed from docs/quality-system to main August 4, 2026 11:34
Thirty-two published videos were the raw render output. One 20-second catalog
preview was 60 MB at 25 Mbps — Blu-ray bitrate for a screen capture in a docs
page. Nothing autoplays, so no page shipped that silently, but a reader who
pressed play paid for it.

Re-encoded at 1280 wide, CRF 28, faststart, keeping audio where it existed.
Measured PSNR against the originals at ~38.5 dB, which is visually identical —
the first check I ran returned nothing and I mistook that for a pass, so this
number comes from a filter that actually worked.

Worth correcting one thing I said earlier: the catalog is not systemically
over-encoded. 190 videos, 0.36 GB, about 1.9 MB each. Six were over 10 MB. I
found two monsters and generalised from them.

Pages carrying more than 8 MB of video: 26 before, and the heaviest went from
140 MB to under 15 MB.

The durable fix is in generate-catalog-previews.ts, which had no delivery encode
at all — it published the render master directly. It has a web pass now, so the
next preview is born small instead of being cleaned up later.
@mintlify

mintlify Bot commented Aug 4, 2026

Copy link
Copy Markdown

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

Project Status Preview Updated (UTC)
hyperframes 🟢 Ready View Preview Aug 4, 2026, 6:35 PM

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

My catalog rebuild added `poster="<name>.png"` to every preview. Thirteen items
have an .mp4 but no .png, so the page asked for an image that does not exist —
and a browser fetches the poster before the video, so those were thirteen 403s
on load. The merged version did not have this; I introduced it.

The generator checks the file now, the same way it already checks before telling
a reader to read a comment header that may not be there. Thirteen pages lose the
attribute; the videos are unaffected.

Also uploads the three weekly changelog re-encodes to the path they are actually
served from — hyperframes/changelog-videos/, not the docs image tree — which I
had got wrong, leaving three dead links in the previous commit.

All 597 media URLs on this branch now return 200.
The previous commit claimed all 597 media URLs returned 200. They did not — 13
still 403'd, and I pushed that claim without re-running the check after the fix.
The message was wrong; this is the correction.

Two files carry a poster, not one. Guarding the page generator left
catalog-index.json still pointing at 13 .png files that were never produced,
which is a broken thumbnail in the catalog grid rather than a missing one.
catalogPreviewFor now returns undefined when the file is absent, the same rule
the page preview uses, and the 13 stale entries are dropped from the written
index.

Verified after the change, not before: 584 media URLs on this branch, 0 dead.
Measured across all 136 hand-written pages, the twelve hardest to read were all
in prompting/. These ten were untouched — 29% to 40% of their sentences ran over
28 words, against a site median under 15%.

  media-and-audio           39.7% -> 6.2%    15 semicolons -> 6
  visual-specs              37.1% -> 19.0%   11 -> 9
  weekly-updates            39.1% -> 0.0%
  runtimes-and-3d           34.1% -> 17.5%
  design-systems            34.0% -> 4.3%
  recreating-references     33.3% -> 7.3%    16 -> 1
  storyboards               31.9% -> 5.4%
  variables-and-templating  30.3% -> 7.3%
  generated-artwork         29.4% -> 9.1%
  explainers                29.1% -> 3.3%    12 -> 1

Word counts barely move — 1752 to 1748, 1633 to 1617 — which is the point. The
facts and worked examples all survived; the sentences carrying them got shorter.
weekly-updates is an archive, so no date, version or fact changed there at all.

One anchor came back: capstone#the-full-prompt-verbatim, which these pages link
to and which an earlier commit had already moved to #the-prompt-word-for-word.
Repointed. Branch total: 0 broken pages, 0 broken anchors.
Product launch, colour grading, faceless explainer, captions and recuts, motion
graphics, and voice and audio.

Five of these led with a six-to-eight second teaser. A six-second clip on the
page that teaches you to make explainers does not teach anything — it is a
placeholder that looks like content. Colour grading, the most visual feature in
the product, had no video at all.

The new films are 34 to 41 seconds and each shows a RANGE rather than one look:
four ways to tell the same idea with no footage; five caption treatments on one
untouched clip; correction, presets, curves, selective colour, all four scopes
and a LUT; narration with music ducking under it and effects on the beat. The
launch film is built by running the workflow on huly.io for real, so the page
that sells the workflow is showing its actual output.

They share one visual system, measured from HyperFrames own light-theme launch
films rather than invented: bone field, near-black type, coral accent, and the
product always a dark panel inset on the light field. Every one carries audio at
about -20 dB and is 1-2 MB.

The old teasers are removed from the three pages where they became a duplicate.
They stay in the workflow picker, which is where a six-second tile belongs.

Guides with nothing to look at: 15 at the start of this work, 5 now.

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

Re-review of exact head aa49f7d924985dacc76f4a820fa2470d6d1853ad.

The prior Catalog-tail, post-footer preservation, GSAP error scoping, unlink cwd, lazy-loading, main rebase, and shortcuts keep-both-sides findings are fixed. The following source-verified blockers remain:

  1. Reduced-motion users still cannot voluntarily play every preview (docs/snippets/hover-video.jsx:76-103, docs/snippets/replica-compare.jsx:100-147). HoverVideo only renders a play control when hasAudio is true, but 12 Thirty Days cards now use hasAudio={false}; under reduced motion those cards have no src and no control. ReplicaCompare also removes both sources when reduced motion is active while leaving its sound button enabled, so the visible control calls play() on media with no source. Keep autoplay disabled, but expose a voluntary load/play action on both surfaces.

  2. The new universal can() guarantee is false (docs/sdk/reference/edit-operations.mdx:79, repeated near line 321). CompositionSession._dispatch calls applyOp directly; it does not run validation. A missing-target addGsapTween fails can() with E_TARGET_NOT_FOUND, while dispatch falls back to a selector and can still write the tween/script. Scope the no-op claim specifically to the no-timeline addGsapTween/addLabel cases, or enforce validation in dispatch before documenting the broader invariant.

  3. The timing guide still describes shipped code as future work and demonstrates an unreachable error (docs/sdk/guides/timing-and-animation.mdx:187-208). The parser is already used by document.ts, and E_NO_GSAP_TIMELINE is emitted only for addGsapTween and addLabel; the example calls setGsapTween. Rewrite the section and remove the matching stale future-parser comment in packages/sdk/src/types.ts:595.

  4. data-playback-start is still documented too narrowly (docs/reference/html-schema.mdx:125). A lone value is ignored by the generic parser/compiler, but runtime playback and Studio trim/split paths do honor and preserve it (timelineEditingHelpers.ts, sourceMutation.ts). The categorical “will not shift trim/split timing” statement is false; scope the limitation to the generic parser/compiler.

  5. The acknowledged Usage carry-forward collision remains (scripts/generate-catalog-pages.ts:138-144). Ordinary human prose beginning with a historical generated opener is still classified as generated and silently deleted on regeneration. Replace the opener heuristic with explicit generated-region/version ownership and add an executable preservation fixture.

CI is otherwise healthy: all 7 required contexts are green and all relevant suites are green except one non-required regression shard still running at review time.

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

Round 2 at aa49f7d9. Additive to the CHANGES_REQUESTED already on the PR — not stacking a second one. We converged on the dispatch() item independently; below is the demonstration and the exact replacement wording, plus one correction to my own round-1 verification and a few things the other review doesn't cover.

Correction: I was wrong about data-playback-start

I verified that item in round 1 and would have marked it closed. That was an incomplete check and the other review is right to flag it.

What I checked and what holds: htmlParser.ts:375 and timingCompiler.ts:140/:287 read data-media-start and never data-playback-start. What I didn't check is the editor, which is where trim and split actually happen for a user:

  • packages/studio/src/player/lib/timelineElementHelpers.ts:156-165 builds the timeline model from data-playback-start first and falls back to data-media-start — and the fallback variable is literally named legacyMediaStartValue.
  • packages/studio/src/hooks/timelineEditingHelpers.ts:186-207 is the trim path. resolveResizePlaybackStart() reads playback-start ?? media-start, applies the trim delta, and writes back through playbackStartAttributeForElement(), which returns data-playback-start for any element authored with it and for every kind === "composition".
  • packages/cli/src/commands/snapshot.ts:426 reads dataset.playbackStart ?? dataset.mediaStart too.

So "a lone data-playback-start will not shift trim/split timing" is false in the editor, and "runtime-only alias" is not accurate either — Studio and the CLI both read it, and Studio writes it. The new wording also inverts Studio's own sense of the pair: it tells readers to prefer the attribute Studio's source calls legacy.

The honest version of this row is that the two names are read by different layers with different precedence — the compile path reads only data-media-start, while the runtime, Studio, and the CLI read data-playback-start first — and that authoring both is what actually causes divergence.

My round-1 op-scope finding had a second site I missed

Same defect, different page, found in the other review: docs/sdk/guides/timing-and-animation.mdx:185-200 gates a setGsapTween call on E_NO_GSAP_TIMELINE, which that op cannot return, and introduces it as needing "the parser engine, which ships in a later phase" — shipped code. I found the defect on edit-operations.mdx and never grepped for the code's other occurrences, which is what would have surfaced this. The page is in this PR's diff.

The dispatch() sentence — demonstrated, and a suggested replacement

Both notes now carry a guarantee:

docs/sdk/reference/edit-operations.mdx:79 — "Don't dispatch() after a failed can() — a rejected op applies nothing (no patches, script unchanged)."

:321 — "A failed can() means dispatch() applies nothing."

The structural reason it can't hold: Session._dispatch() calls applyOp(this.parsed, op) directly (packages/sdk/src/session.ts:608), and applyOp (packages/sdk/src/engine/mutate.ts:275) contains no can() call at all. Whether a rejected op is a no-op is a property of each individual write helper, not a contract of dispatch().

And for the two ops this note names, there is a concrete input where can() rejects and the op still mutates. When a script calls tl.* but has no gsap.timeline() declaration, parseGsapScriptAcornForWrite falls back to timelineVar: "tl" with hasTimeline: false (packages/parsers/src/gsapParserAcorn.ts:1829-1845), so can() returns E_NO_GSAP_TIMELINE — while findInsertionPoint still returns a real offset, because parsed.located is non-empty (gsapWriterAcorn.ts:309-316). Running the real helpers against tl.to(".box", { x: 100 }, 0);:

  • addLabelToScript(script, "intro", 0) → appends tl.addLabel("intro", 0);
  • addAnimationToScript(...) → appends tl.to(".other", { y: 25, duration: 0.5 }, 1);

Both after can() said no.

Calibration: no HTML in the repo's own examples or fixtures has tl.* calls without a gsap.timeline() declaration, so this breaks nobody today.

Suggested wording, which avoids needing a per-op carve-out and won't have to be revisited when a helper's no-op behaviour changes: dispatch() does not consult can(). Call can() first and skip the op when it fails.

Small addition on the two snippets

Beyond the live-button-with-no-source point already raised: the two sound buttons have also drifted apart on accessibility in this same round. hover-video.jsx gained aria-pressed={!muted} and a focus-visible:outline treatment; replica-compare.jsx's button has neither. Worth bringing them to the same shape while both files are open.

Verified and closed

  • Catalog and doctrine. 169/169 catalog pages carry ## Related topics again; guides/ 32/32 and studio/ 13/13 unchanged. You took the option I'd have picked — restore the emission rather than amend docs/AGENTS.md:61 — so the doctrine file needed no edit. The restored bullets are byte-identical to the pre-PR ones, so this is a real restore and not a same-shaped substitute. Regeneration is byte-clean: I ran the generator and git status came back empty.
  • The carriedSectionsFrom half, which is the part with no test: I appended a ## Field notes section below the generated tail on a catalog page and regenerated. It survived, and it was relocated above the footer marker so ## Related topics is still the last heading. Both halves hold. Worth pinning — the committed-output assertion you added catches a dropped section, but nothing covers the carry-forward path, which is the subtler of the two. That assertion also walks blocks/ and components/ only, so the catalog index page isn't covered by it (it does carry the section).
  • bun unlink. The absolute path fixes it, and the whole sequence now tracks: "From the repository root" → cd packages/clicd /path/to/a/video-project → absolute path back.
  • Style note. Imported rather than copied, and the string is byte-identical to the literal it replaced, so the gate's behaviour is unchanged. The import is side-effect-free — set-version.ts:381 guards main() behind the entrypoint check — and introduces no cycle.
  • GSAP op scope on edit-operations.mdx. :79 names both ops, :321 correctly excludes setGsapTween / removeGsapTween / removeGsapProperty, and the Labels section carries the requirement. removeLabel is correctly excluded — it resolves through gsapScriptMissing(parsed) ?? CAN_OK with no timeline check (mutate.ts:1773). The guide page above is the remaining site.

The rebase

Exactly one file collided across 278 versus 138, and it resolved as a true union. Both directions:

  • merged docs/studio/shortcuts.mdx against main's version → only your edits (Ctrl + Y, Delete / Backspace, the rewritten paragraph, the new J / Shift + J row)
  • merged against your pre-merge version → only main's 18-line ## Timeline navigation section

Nothing dropped either way, and main's tip is now an ancestor of the head.

One consequence worth a look, since it's the kind of thing this PR exists to fix: your rewritten paragraph says arrow keys either nudge a selected canvas element or step through frames, and it now sits directly above a merged-in section documenting a third arrow-key behaviour — moving between timeline targets when the timeline has focus. Both are accurate alone; together, "the selected area decides" reads as exhaustive when it isn't.

Nav, links, CI

  • Nav invariants identical to main: 308 nav pages, 29 redirects, 0 collisions, 0 dangling, 0 dupes.
  • 1313 internal links across the 250 changed docs files, 0 unresolved. All three new ## Related topics destinations exist (/catalog, /studio/assets-and-blocks, /go-further), which is what the 507 new links ride on.
  • CI at aa49f7d9: 58 success, 2 skipped, 0 failures. There are 8 required contexts, not 7 — regression is one of them, and it's the roll-up the shards feed, so it was genuinely outstanding while shard-5 was running. It has since passed, so all 8 are now reported and green. Neither skipped check is required. BLOCKED here is the review gate, not CI.

Out of scope, worth naming once

The autoplay guard now covers 53 hand-written cards (30 in thirty-days.mdx, 23 in examples.mdx), with 0 raw <video autoPlay> left on either page. The catalog generator still emits raw <video autoPlay muted loop playsInline> on 167 pages. That count is identical at the merge-base, at main, and here, so it isn't this PR's regression and I wouldn't widen this PR for it. But it's the same hand-written-versus-generated split as the ## Related topics one: the standard gets set on pages a human edits, and the generator keeps doing its own thing.

I don't merge these, and it stays blocked on the live changes-request regardless — that one is for the reviewer who filed it to clear.

Review by Rames Jusso

…tart layers

- edit-operations: dispatch() does not consult can() (session.ts:608 -> applyOp
  with no validation; a no-timeline addGsapTween/addLabel is a no-op but a missing
  target still writes via selector fallback). Use Rames's wording: call can() first
  and skip on failure — no false 'applies nothing' guarantee.
- timing-and-animation: the second E_NO_GSAP_TIMELINE site — gated setGsapTween on
  an error it cannot return and called shipped parser code 'a later phase'. Rewrote
  to gate addGsapTween (which can return it); dropped the stale can() comment in
  types.ts:595.
- html-schema: data-playback-start is read by runtime, Studio and CLI (Studio also
  writes it, timelineEditingHelpers.ts); only the compile path is media-start only.
  Document the layered precedence instead of calling it runtime-only.

Round-2 findings from Magi (#2/#3/#4) and Rames.
…arity

Both snippet players now keep a source assigned whenever in view (preload='none'
so nothing downloads until asked), gating only autoplay on reduced motion — so a
reduced-motion visitor can press play/unmute instead of being left with an inert
poster. HoverVideo renders a control on hasAudio={false} cards too (a play/pause
toggle), and ReplicaCompare no longer calls play() on a source it just removed.
Gave ReplicaCompare's button the same aria-pressed + focus-visible as HoverVideo.
Round-2 finding from Magi (#1) and Rames.
carriedSectionsFrom() decided whether a ## Usage section was generated by matching
its first line against a list of historical opener phrases — so a hand-written
Usage section that happened to open that way was classified as generated and
silently deleted on regeneration. Ownership is now purely set membership: a
section is generated iff its heading is one the template emits, and ambiguous
'usage' is no longer in that set (the template never emits it), so any ## Usage is
carried. Exported carriedSectionsFrom behind an entrypoint guard and added two
executable preservation fixtures. Flagged by Magi (#5).
… navigation

The merged '## Timeline navigation' section documents a third arrow-key behaviour;
the 'selected area decides' paragraph read as exhaustive with only two. Fold in the
timeline-focus case and link the section. Coherence note from Rames.

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

Round 3 is materially better. I verified the catalog carry-forward change against the generator, ran the focused 172-test catalog suite, regenerated all 168 pages byte-clean, and ran both the live snippet-motion gate and its 10 focused tests. The explicit heading ownership closes the destructive Usage heuristic, the post-footer fixture covers the formerly untested path, the dispatch() wording now matches _dispatch → applyOp, both GSAP sites are scoped to addGsapTween/addLabel, and the shortcuts paragraph now coexists cleanly with Timeline navigation.

Two blockers remain:

  1. docs/snippets/replica-compare.jsx:72-104 — reduced-motion voluntary playback still only starts the left/reference film. The synchronization effect returns early when reduced is true, and toggleSound() calls a.play() only. The right/replica film therefore remains paused, so a reduced-motion visitor cannot actually view the comparison pair after pressing the sole control. Keep autoplay disabled, but make the explicit user action start/pause both films (and preserve their synchronization), with a focused reduced-motion assertion.

  2. docs/reference/html-schema.mdx:125 — the CLI claim still contradicts the shipped readers. packages/cli/src/commands/validate.ts:194 reads only data-media-start; the producer/compiler paths likewise read data-media-start, while the core runtime and Studio are the layers that prefer data-playback-start. packages/core/scripts/debug-timeline.ts:156-157 is not a fallback implementation either: it reads playback-start only. Please describe each layer precisely instead of grouping the CLI with runtime/Studio.

CI at review time has no failures, but the two Windows required contexts and several regression/integration jobs are still running.

Verdict: REQUEST CHANGES

Reasoning: The generator, GSAP, dispatch, and shortcuts fixes are source-correct and locally verified, but the reduced-motion comparison remains functionally incomplete and the schema still makes a source-contradicted compatibility promise.

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

Round 3 at b541245f. Additive to the CHANGES_REQUESTED already on the PR — not stacking a second one. I reached the html-schema item independently; rather than restate it, below is the full layer map from source, including one reader in neither half of the current sentence, plus the element-kind rule I think the row actually needs.

The html-schema row — one more layer, and the rule that follows from it

Agreed the row is still contradicted. The part I'd add: the split isn't compiler-vs-everything-else, and the layer where it bites hardest isn't named on either side.

Reads only data-media-start:

Layer Site
Timing compiler packages/core/src/compiler/timingCompiler.ts:140, :287
HTML parser packages/parsers/src/htmlParser.ts:375
CLI validate packages/cli/src/commands/validate.ts:194 → duration math at :211
Engine audio mixer packages/engine/src/services/audioMixer.ts:350 → ffmpeg -ss at :442

Reads data-playback-start first, falls back to data-media-start:

Layer Site
Runtime player packages/core/src/runtime/media.ts:18, :92
Studio packages/studio/src/player/lib/timelineElementHelpers.ts:156-165 (and writes it back)
CLI snapshot packages/cli/src/commands/snapshot.ts:426

Two things fall out of that:

The CLI is on both sides. validate reads only data-media-start; snapshot prefers data-playback-start. So "the CLI reads data-playback-start first" is true of one command and false of the other — naming the commands is the only way that clause survives.

The audio mixer makes the failure a shipped artifact. parseAudioElements reads data-media-start with no fallback and feeds it straight to -ss. So on a <video> authored with only data-playback-start: the player and snapshot trim the picture, and the mix seeks to 0. The rendered file has trimmed picture over untrimmed audio — silent at author time, visible only in the output. (packages/producer/src/services/audioExtractor.ts:54 has the same media-start-only read, but I found no caller for it; the engine copy is the live path.)

Which is why "Set one" doesn't resolve it — which one depends on the element kind:

  • <video> / <audio>data-media-start. It's the only name the compiler, validate, and the audio mixer read.
  • Nested composition → data-playback-start. Studio writes it (kind === "composition"), and concepts/compositions.mdx:107 already documents it as the child-timeline offset.

That reading also makes the two pages in this PR agree with each other, which "set one" doesn't quite do today.

Same claim on a third page, outside this diff

docs/prompting/editing-existing-videos.mdx:21 maps "skip the first second of the clip" → Trim (front, media only)`data-media-start` / `data-playback-start`. The Note directly under it scopes front trim to <video>/<audio> — so this row offers the two names as interchangeable for the exact element kind where they aren't. Not in the diff, and it's one table cell; flagging it because it's the page a reader hits before the schema reference, and it would leave the docs disagreeing with themselves right after a PR that exists to fix that.

dispatch() / can() — your correction to my round-2 example is right

Your parenthetical is more accurate than what I demonstrated. I ran all four shapes through openCompositioncan()dispatch() at this head:

Script / op can() dispatch()
Timeline present, target id doesn't exist E_TARGET_NOT_FOUND writes tl.to("[data-hf-id=\"hf-does-not-exist\"]", …)
No timeline, no tl.* calls E_NO_GSAP_TIMELINE no-op
No timeline decl but tl.* calls present E_NO_GSAP_TIMELINE writes
No GSAP script at all E_NO_GSAP_SCRIPT throws Error

Row 2 is the realistic no-timeline shape and you're right that it no-ops — my round-2 example was row 3, which is the contrived one I'd already calibrated as absent from the repo's fixtures. Row 1 is the better demonstration: an ordinary composition and a typo'd id, and gsapTargetSelector (mutate.ts:1215) falls back to [data-hf-id="…"] for an element that resolved to null, so the tween lands pointing at nothing.

Row 4 is worth knowing since it argues for the wording you landed: skipping can() doesn't only risk a bad write, it can throw — handleAddGsapTween (mutate.ts:1232) throws on a missing script where can() would have returned a code. "dispatch() does not consult can() — call can() first and skip the op when it fails" covers all four rows without a carve-out, which the previous guarantee couldn't.

Confirming the other open blocker

Verified at source, so it isn't two reviewers asserting the same thing from the same text: replica-compare.jsx:72 returns early on reduced, so the sync effect never attaches, and toggleSound (:104) plays only refVideo. Under reduced motion the replica stays paused and the side-by-side — the whole point of the component — can't be viewed. hover-video.jsx doesn't have this shape because togglePlay drives the single element directly.

Verified and closed

  • Catalog generator. 172 tests pass; full regen is byte-clean (git status empty after running it). 168/168 catalog pages carry ## Related topics, guides/ 32/32, studio/ 13/13.
  • The ownership rewrite is safe today, and I checked the case that would have made it unsafe. Dropping usage from GENERATED_HEADINGS means any ## Usage on an existing page is now carried forever — so a generator-written Usage left over from an older revision would get pinned as hand-written. There are 0 catalog pages with a ## Usage section, and the current template emits none, so there's nothing to mis-adopt.
  • Carry-forward, end to end through the real generator (the fixtures cover the unit; this covers the integration): appended a ## Field notes section below the footer marker and a ## Usage opening with After installing, add the block to a real catalog page, then regenerated. Both survived verbatim, both were relocated above {/* hf:generated-footer */}, generated Usage prose stepped aside with no duplicate heading, and ## Related topics is still the last heading. Restored, regen still byte-clean.
  • timing-and-animation.mdx. The op now gated is one that can actually return the code, and the example type-checks against the real op shape — { type: "addGsapTween"; target: HfId; tween: GsapTweenSpec } (types.ts:144), with method / properties / duration all on GsapTweenSpec (:292). The types.ts:595 comment no longer promises a future phase for shipped code.
  • shortcuts.mdx. ## Timeline navigation exists at :47, so #timeline-navigation resolves, and the paragraph no longer reads as exhaustive above it.
  • Rebase. origin/main is still an ancestor of the head — 0 behind, no new collisions since the merge.
  • Nav and links. 308 nav pages, 29 redirects, 0 collisions / dangling / dupes — identical to main. 1313 internal links across the 250 changed docs files, 0 unresolved.

CI

At b541245f: 46 success, 2 skipped, 10 running, 0 failures. Of the 8 required contexts, 5 are reported and green (Build, Typecheck, Test: runtime contract, Render on windows-latest, Semantic PR title); Tests on windows-latest is running, and Test and regression haven't reported yet — regression is the roll-up 8 of 9 shards still feed. Neither skipped check is required. Nothing failing, but it isn't 8/8 yet.

One structural note

catalogGeneratorInstructions.test.ts importing ../../../../scripts/generate-catalog-pages.ts is the first test under packages/ to import from the repo-root scripts/ directory. Typecheck and Build are both green so it's fine as shipped; worth knowing that packages/core's suite now needs the repo root present to run.

I don't merge these, and it stays blocked on the live changes-request regardless — that one is for the reviewer who filed it to clear.

Review by Rames Jusso

…start layers

- replica-compare: the voluntary control now starts and pauses BOTH films (not
  just the reference), and the replica-sync effect attaches in view regardless of
  the preference, so a reduced-motion visitor who presses play sees the whole
  synchronized pair. Added a focused source-level assertion to the motion-check
  suite (the repo has no React runtime harness for snippets).
- html-schema: describe each layer precisely instead of grouping the CLI —
  timing compiler, HTML parser, producer audio, and 'hyperframes validate' read
  only data-media-start; runtime, Studio and 'hyperframes snapshot' read
  data-playback-start first (Studio also writes it).

Round-3 findings from Magi.
Per Rames: add the engine audio mixer (audioMixer.ts:350 -> ffmpeg -ss) to the
media-start-only readers — it's the live path that makes a lone data-playback-start
a shipped output bug (trimmed picture over untrimmed audio). 'Set one' isn't enough;
give the element-kind rule: <video>/<audio> use data-media-start, nested composition
uses data-playback-start (Studio writes it; it's the child-timeline offset). Also fix
editing-existing-videos.mdx, which offered both names as interchangeable for front
trim of a clip — the exact kind where they aren't.

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

The two round-3 blockers are fixed on their primary paths. docs/snippets/replica-compare.jsx:68-120 now attaches synchronization under reduced motion and explicitly starts/pauses both films; the 11-test motion suite and live snippet audit pass. docs/reference/html-schema.mdx:125 now matches the split readers, including validate, snapshot, and the audio mixer, and docs/prompting/editing-existing-videos.mdx:21 correctly makes media front-trim use data-media-start.

One same-contract blocker remains:

  • [blocker] docs/snippets/replica-compare.jsx:54-66,111-120 — the offscreen cleanup releases both sources but does not restore muted state. Reproduce: under reduced motion (or keyboard/touch), press the button to unmute/start the pair, scroll it out of view, then return. React still has muted === false; the control says “Sound on,” while both films were paused and unloaded. On re-entry, a permissive browser can attempt audible autoplay; a restrictive browser rejects it and leaves the pair paused. The first press then mutes/pauses rather than playing. HoverVideo already closes the same lifecycle at its offscreen branch with setMuted(true). Reset the reference and React state when releasing the sources, and pin the out-of-view → in-view state transition (the current source-level assertion covers initial start/sync only).

Required CI is also not terminal yet: both Windows contexts are still running and the remaining required rollups have not reported.

Verdict: REQUEST CHANGES

Reasoning: The requested reduced-motion and media-offset corrections are source-correct, but the same reduced-motion control breaks after its normal viewport-release lifecycle and can re-enter with stale audible state.

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

Round 4 at c49fea2e0. Both fixes verified at source, and the html-schema rewrite is accurate — including the element-kind rule, which I checked properly rather than accepting because it started as my suggestion. Additive to the CHANGES_REQUESTED already on the PR; the findings below are test-hardening and precision, not a second blocker.

The new test passes, but it doesn't pin the thing it's named after

The assertion is titled "the control starts both films," and the behaviour it actually guards is narrower than that. I mutation-tested it — three edits, each run against check-docs-snippet-motion.test.mjs:

Mutation Result
Re-add reduced to the sync-effect guard 10 pass / 1 fail — caught
Delete repVideo.current?.play() from startBoth 10 pass / 1 fail — caught
Replace startBoth() in toggleSound with a.play().catch(() => {}) — helper left defined, call site reverted to reference-only 11 pass / 0 failnot caught

The third one is the exact regression this round fixed. assert.match(source, /repVideo\.current\?\.play\(\)/) is satisfied by the helper's body, which stays in the file whether or not anything calls it, so the test verifies that startBoth exists rather than that the control uses it. Both other assertions are wired to the right thing; this is the one gap, and it's the one that matters.

Staying source-level (agreed on the reasoning there), one more assertion closes it — scope the match to toggleSound's body rather than the file:

const toggle = source.match(/const toggleSound = \(\) => \{([\s\S]*?)\n  \};/);
assert.ok(toggle, "found toggleSound");
assert.match(toggle[1], /startBoth\(\)/, "voluntary play goes through startBoth");

That fails on the third mutation and passes at this head — I ran both.

The element-kind rule holds, and the row can say why in one clause

I suggested this rule, so I verified it rather than reading it back. The question it has to survive is the obvious one a reader will ask: if the timing compiler reads only data-media-start, why am I told to put data-playback-start on a nested composition?

The answer is that none of the four media-start-only readers ever look at a nested composition:

  • Timing compilercompileTag and extractResolvedMedia both match on /<(?:video|audio)[^>]*>/gi. Composition hosts never enter either.
  • HTML parser — the composition branch builds compositionElement with no media-start read at all (htmlParser.ts:352-370); the data-media-start read is in the else branch, behind MEDIA_TYPES.has(type) (:372).
  • Audio mixer — selects audio[id][src] and video[id][src][data-has-audio="true"] (audioMixer.ts:365, :371).
  • hyperframes validate — selects audio[data-duration] (see below).

So the rule strands no reader: for a composition, the media-start-only group reads neither name, and data-playback-start is the only name anyone reads. Adding a half-sentence to that effect — "those four only ever inspect <video> / <audio>, so a composition's offset is never theirs to read" — turns what currently looks like a contradiction into a rule that explains itself.

Also worth one clause, because it's what makes the advice stable rather than merely correct today: Studio writes back whichever name it found (timelineEditingHelpers.ts:182 returns data-playback-start for playbackStartAttr === "playback-start" or kind === "composition", and data-media-start otherwise), and compositionInsertion.ts:209 stamps data-playback-start on inserted composition hosts. Both halves of the kind rule round-trip through an editing session instead of drifting to the other name.

hyperframes validate is audio-only

auditClipDurations has one media selector — querySelectorAll("audio[data-duration]") (validate.ts:145) — and it's the only element selection in the command apart from the composition root. The kind: el.tagName === "AUDIO" ? "Audio" : "Video" ternary at :192 can only ever produce "Audio"; it's vestigial (pre-existing, and not something to fix in a docs PR — noting it because it's what makes the selector easy to misread).

Naming validate in the media-start-only group is accurate, but it invites the inference that the tool checks video offsets. It doesn't: the failure this row now warns about — a <video> with only data-playback-start, trimmed picture over untrimmed audio — is invisible to hyperframes validate. Given the row goes out of its way to name the CLI split, "hyperframes validate (which only inspects <audio>)" is worth the four words, because the practical question a reader has after reading the warning is will tooling catch this for me, and the answer is no.

Related, same sentence: snapshot selects document.querySelectorAll("video") (snapshot.ts:418). So the two commands inspect disjoint element sets and never actually disagree about the same element — "the CLI is split" is true of the codebase but slightly overstates the hazard.

The offscreen-state blocker — confirmed from source

I reached the same thing independently, so as a second data point rather than a second assertion of the same text: hover-video.jsx:52 calls setMuted(true) inside its !inView teardown, immediately after removeAttribute("src") + load(). replica-compare.jsx's teardown (:54-62) does the same three DOM calls on both elements and omits the state reset. After play → scroll out → return, muted is still false while both elements sit at time 0 with the label reading "Sound on"; the next press takes the next = true branch and mutes/pauses rather than playing. It's a one-line asymmetry with the sibling component. That one's for the reviewer who filed it.

One small thing on the control's accessible name

Now that the button starts and pauses the pair under reduced motion, its name describes less than it does. aria-label is "Mute the reference" when unmuted — but under reduced motion pressing it runs pauseBoth(), stopping the whole comparison. Same control, accurate name in one motion mode and not the other, and the mode where it's wrong is the accessibility one. If the reduced branch keeps the pause, the label wants to follow it (reduced ? "Pause the comparison" : "Mute the reference", and symmetrically for the play direction, which now starts both).

Verified clean

  • Third page. editing-existing-videos.mdx:21 now names data-media-start alone, and that's coherent with the Note directly under it, which scopes front trim to media clips. The page has no composition-trim verb, so dropping the second name leaves no gap.
  • The three pages agree. concepts/compositions.mdx:80 (data-playback-start="0" on a nested composition) and :107 (child-timeline offset) now line up with the kind rule instead of sitting beside a "the two names are interchangeable" claim.
  • Links. 1321 internal links across 251 changed docs files, 0 unresolved — the new /concepts/compositions link resolves.
  • Nav. 308 pages, 29 redirects, 0 collisions / dangling / dupes — identical to main.
  • Motion gate. 11/11.
  • Rebase. origin/main still an ancestor, 0 behind. The diff since b541245f is 4 files / +44 −5 and touches nothing outside the two items.

CI

Not green yet — and further from it than the last head, because the push reset everything. Of the 8 required contexts, none has reported green: Render on windows-latest and Tests on windows-latest are pending, and Semantic PR title, Typecheck, Build, Test, Test: runtime contract, and regression haven't reported at all (all 9 regression-shards are still pending, and regression is the roll-up they feed). 19 non-required checks pass, 1 skipping, 0 failures. Expected for a head this new — just worth not reading "0 failures" as "green."

I don't merge these, and it stays blocked on the live changes-request regardless.

Review by Rames Jusso

…y labels

- replica-compare: the offscreen teardown now resets muted (element + React state),
  matching HoverVideo — a pair unmuted before it scrolled away no longer returns
  reading 'Sound on' over a paused, sourceless pair (Magi blocker).
- Both controls' aria-labels now follow the mode: under reduced motion the button
  plays/pauses the whole comparison, and when a preview is already autoplaying
  muted the action is 'unmute', not 'play with sound' (self-review + Rames).
- Hardened the motion-suite assertion to scope 'startBoth' to toggleSound's body
  (a defined-but-unused helper no longer satisfies it) and pin the offscreen
  muted-reset transition (Rames mutation-test gap).
- html-schema: 'hyperframes validate' inspects <audio> only; note that no
  media-start-only reader inspects a composition host, so the kind rule strands
  nobody (Rames).
Grid cards with a public source are wrapped in an <a>; the sound/play button
lives inside it, so a click — mouse or keyboard — bubbled up and navigated to
GitHub instead of toggling sound. preventDefault + stopPropagation on the control.
Found by driving the live preview (self-review).

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

Round 5 at 83db94e7. Both of my round-4 items landed, and I checked them by mutation rather than by reading: reverting the toggleSound call site now fails 10/1, as does dropping setMuted(true) from the teardown. The validate scoping is in. One correction below on the clause I asked for — the advice is right, the reason given for it isn't — plus one coverage gap on the bug you found yourself.

The composition clause is stronger than the source supports

none of the media-start-only readers inspect a composition host, so its offset is only ever read as data-playback-start

First half: correct, and I verified each one. Second half isn't. The reader that resolves a composition host's offset is readElementPlaybackStart (media.ts:16-21):

export function readElementPlaybackStart(el: Element): number {
  const raw = Number.parseFloat(
    el.getAttribute("data-playback-start") ?? el.getAttribute("data-media-start") ?? "",
  );

It takes Element, not HTMLMediaElement, and timeline.ts:415 calls it on every clip it builds — including the branch that computes kind: "composition" twenty lines earlier (:391-399). Studio does the same without a kind gate (timelineElementHelpers.ts:157-159: playbackStartValue ?? legacyMediaStartValue). So data-media-start on a composition host is read; it isn't ignored, and a reader who takes that sentence literally will conclude something false about a composition that already carries the legacy name.

The accurate reason is nearby and still supports the same rule:

— a composition host is only ever looked at by the playback-start-first group, and data-playback-start is the name Studio writes and normalises to, so authoring the other name works but won't survive an edit.

Worth fixing precisely because the rest of this row now earns its length by being exact. (Every media-start-only reader really is media-scoped, all four: compiler matches /<(?:video|audio)[^>]*>/gi; parser's composition branch has no offset read; mixer selects audio[id][src] + video[…][data-has-audio="true"]; validate selects audio[data-duration].)

The navigation fix is the one thing in the PR with no coverage

Mutation battery against the gate at this head:

Mutation Result
startBoth()a.play() in toggleSound 10 / 1 — caught
Drop setMuted(true) from the offscreen teardown 10 / 1 — caught
Drop e.preventDefault() + e.stopPropagation() from HoverVideo 11 / 0 — not caught
Drop video.muted = true from the teardown loop 11 / 0 — not caught (fine: React's muted prop re-asserts it on the re-render setMuted(true) triggers)

The third row is the defect that survived two rounds of static review and the gate, and only turned up because you drove the live preview. That makes it the single best candidate for an assertion, not the one to leave unpinned — same source-level style as the others:

const onClick = hoverSource.match(/onClick=\{\(e\) => \{([\s\S]*?)\n {8}\}\}/);
assert.ok(onClick, "found the control's click handler");
assert.match(onClick[1], /stopPropagation\(\)/, "control must not bubble to a wrapping link");

Blast radius, since "the card's <a> source-link" undersells it. HoverVideo appears 53 times, and 40 of those are inside a link: 11 of the 23 cards on examples.mdx are <a href> (the other 12 are <div>), and 29 of the 30 on thirty-days.mdx are <Card href> — a page that states outright, at :18, "Every card opens the original post." So the fix is at the right level: patching the examples.mdx call sites would have left all 29 of the thirty-days ones broken.

ReplicaCompare correctly not touched here — I checked instead of assuming symmetry. Its wrapper at examples.mdx:148 is a plain <div className="not-prose my-6 space-y-8">, so its button isn't inside a link and needs no suppression today.

Accessible names are now precise; the visible text they're attached to isn't

All four ReplicaCompare names read correctly against the behaviour — reduced really is the discriminator for "is this already autoplaying", since active = inView && !reduced. The wrinkle is that the button's visible label is still {muted ? "Sound off" : "Sound on"} (:174), and none of the four accessible names contains that string. That's the WCAG 2.5.3 (Label in Name) case: a speech-input user saying "click Sound off" gets no match. Either fold the visible words into the name ("Sound off — play the comparison") or make the visible text mode-aware alongside the label. HoverVideo is unaffected — its button is icon-only, so there's no visible text to match.

One structural note, not for this PR

preventDefault + stopPropagation makes the control behave, but the markup underneath is still a <button> inside an <a>, which the HTML content model disallows (an anchor may not have interactive content descendants), and the button stays inside the link's accessible subtree either way. The behavioural fix is the right call for this PR; the root fix is the card markup — link the title rather than wrapping the whole card. Flagging it as a follow-up, not an ask here.

Verified clean

  • Gate. 11/11 at head, and the two assertions added this round fail on the exact regressions they name.
  • Docs. 1321 internal links / 0 unresolved; nav 308 / 29 redirects / 0 collisions / 0 dangling / 0 dupes — identical to main.
  • Rebase. origin/main still an ancestor, 0 behind. Diff since c49fea2e0 is 4 files, +40 −7, nothing outside the four items.
  • Labels. reduced is in scope in both components; the HoverVideo branch matches its own autoplaying = inView && !reduced.

CI

4 of 8 required contexts green (Semantic PR title, Test: runtime contract, Typecheck, Build). Test, Render on windows-latest and Tests on windows-latest are pending; regression hasn't reported. 45 pass / 12 pending / 2 skipping, 0 failures.

Fix the composition clause and I'd have nothing blocking left on my side — the coverage gap and the label point I'd take as follow-ups rather than hold the PR for. Still not mine to merge, and there's a live changes-request in front of it either way.

Review by Rames Jusso

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

Round 5 at exact head 83db94e7.

What improved

The prior blocker is closed cleanly. ReplicaCompare now resets both DOM and React muted state when it leaves the viewport, so it cannot return with a stale “Sound on” control over reloaded muted media. The strengthened motion gate also pins the toggleSound call site and the offscreen state transition; I reran it at this head and it passes 11/11. The nested-link control fix in HoverVideo is at the correct component boundary and the mode-specific labels match the actual play/unmute behavior.

I also reran the Catalog generator suite: 172/172 pass.

Blocking: the composition explanation contradicts the runtime source

docs/reference/html-schema.mdx:125

The kind recommendation is good, but this rationale is false:

its offset is only ever read as data-playback-start

readElementPlaybackStart() accepts any Element and resolves:

data-playback-start ?? data-media-start

See packages/core/src/runtime/media.ts:16-20. packages/core/src/runtime/timeline.ts:400-415 calls that helper for every clip it builds, including clips whose kind is "composition". Studio uses the same fallback without a media-kind gate. A composition host carrying only data-media-start is therefore read; it is not ignored.

The accurate explanation is:

Composition hosts are only inspected by playback-start-first readers. data-media-start is accepted as a fallback, while data-playback-start is the canonical authoring name and the name Studio writes for new composition hosts.

That preserves the useful element-kind rule without claiming the fallback is unreadable.

Important coverage gap

scripts/check-docs-snippet-motion.test.mjs

The live-preview bug found in this round—clicking the HoverVideo control navigating its wrapping card link—is still not pinned. Removing both preventDefault() and stopPropagation() leaves the gate at 11/11. Please add a focused source assertion scoped to the button click handler so this exact regression cannot return. I would not make this alone a merge blocker, but it is unusually high-value coverage because 40 of 53 HoverVideo uses sit inside a link and the bug already escaped static review.

CI

At review time, 6 required contexts are green; Tests on windows-latest and the required regression roll-up remain pending. There are 0 failures.

Verdict: REQUEST CHANGES

Reasoning: The prior UI/state blocker is fixed and locally verified, but the HTML schema now makes a categorical source claim that the shipped runtime directly contradicts. Correct that sentence; the event-suppression assertion is an important hardening item, and terminal required CI still needs to finish.

— Magi

- html-schema: the previous clause claimed a composition host's data-media-start is
  never read. It is: readElementPlaybackStart (media.ts:16) resolves
  data-playback-start ?? data-media-start and timeline.ts calls it on composition
  clips. Rewrote to the accurate reason both reviewers gave — composition hosts are
  only inspected by the playback-start-first readers, media-start works as a
  fallback, but playback-start is what Studio writes/normalises to.
- motion test: pin the HoverVideo click-suppression (preventDefault +
  stopPropagation). Removing it left the gate 12/12; now it fails. This is the bug
  that escaped static review and only surfaced by driving the live preview.
- replica-compare: fold the visible 'Sound off/on' text into the aria-label so the
  accessible name contains it (WCAG 2.5.3, Rames).

Round-5 findings from Magi and Rames.

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

Round 6 at exact head fed2baf2.

What is closed

  • docs/reference/html-schema.mdx:125 now matches the shipped source. Composition hosts are handled by playback-start-first readers, data-media-start is accepted as their fallback, and Studio canonicalizes composition edits to data-playback-start. That is the precise distinction the previous review requested.
  • scripts/check-docs-snippet-motion.test.mjs:165-174 now pins the nested-link event suppression at the actual onClick call site. Exact head passes 12/12; deleting preventDefault() and stopPropagation() produces the expected 11/1 failure.
  • docs/snippets/replica-compare.jsx:151-159 now satisfies Label in Name: each accessible name begins with the visible “Sound off” / “Sound on” text while preserving the mode-specific action.

No implementation blocker remains in those three changes.

Blocking CI: the new test is not formatter-clean

scripts/check-docs-snippet-motion.test.mjs:170

Required regression is red because its preflight runs oxfmt --check ., which reports this file. The exact formatter delta is only the long final assertion:

assert.match(
  onClick[1],
  /e\.stopPropagation\(\)/,
  "the click does not bubble to a wrapping link",
);

Apply oxfmt and push. The regression shards were skipped after preflight failed, so the required roll-up correctly reports failure; this is not an infrastructure flake.

At review time: Build, Semantic PR title, runtime contract, and Typecheck are green; Test is still pending; both Windows jobs are skipped for this head; required regression is failed.

Verdict: REQUEST CHANGES

Reasoning: All three round-five source/test findings are correctly resolved and mutation-verified. The only remaining blocker is the deterministic required-CI formatting failure; format the new assertion and let the exact-head required checks finish.

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

Round 6 at fed2baf2. All three items are correctly closed and I verified each one — details below. On CI I landed on the same root cause as the changes-request already filed (oxfmt rejecting the new assertion), so rather than restate it: below is the exact replacement text, the cascade that connects one line to the red board, and two more required contexts that "apply oxfmt and let CI finish" doesn't cover — they aren't pending, they're skipping.

CI — the wrap, and what else it took down

oxfmt --check fails on exactly one file in the repo (3794 checked), and it's the assertion added this round:

scripts/check-docs-snippet-motion.test.mjs

I ran the formatter, so this is its output rather than a guess — it wants the 103-character call wrapped:

-  assert.match(onClick[1], /e\.stopPropagation\(\)/, "the click does not bubble to a wrapping link");
+  assert.match(
+    onClick[1],
+    /e\.stopPropagation\(\)/,
+    "the click does not bubble to a wrapping link",
+  );

That one line is the root of every red X on the PR, and the reason it reaches regression at all is worth spelling out, because it also explains the two contexts nobody has mentioned. Traced through the logs rather than inferred:

  1. format:check exits 1 → Preflight (lint + format) fails in five workflows (error: script "format:check" exited with code 1), and Format fails on its own.
  2. Every heavy job gates on it — needs: [changes, preflight] in regression.yml:70, windows-render.yml:79 and :380, player-perf.yml:57, preview-regression.yml:64, and needs: preflight in catalog-previews.yml:38 — so all of them skip.
  3. Where there's a roll-up, it fails on the skip: regression's Check-results step is literally if [ "skipped" != "success" ]; then → exit 1, and player-perf says "FAILED — perf-shards result was 'skipped'". Where there isn't one, the context simply never reports — which is the case for both windows jobs.

Net effect on the gate as of writing:

Required context State
Semantic PR title pass
Test: runtime contract pass
Typecheck pass
Build pass
Test pending
regression fail
Render on windows-latest skipping
Tests on windows-latest skipping

The part I'd add to "apply oxfmt, push, let required CI finish": those last two aren't waiting, they're skipped by the same needs: preflight gate (windows-render.yml:79, :380). They will not turn green at this head no matter how long it runs — only the next push re-runs them. So the push isn't just clearing a red X, it's the only thing that gets 3 of the 8 required contexts to report at all.

Two repo-level observations, not yours to fix here. preview-regression's log prints SKIPPED — no preview/runtime changes. Auto-pass. and then exits 1 with PREVIEW_PARITY_RESULT: skipped — the auto-pass branch doesn't actually pass. And more generally, a preflight failure converting into regression: fail rather than regression: skipped means a formatting nit is indistinguishable from a real regression failure at the check-list level. Worth someone's follow-up; it's what made this look like a perf/regression problem at first glance.

The composition rationale is now accurate

Checked clause by clause against source:

  • "composition hosts are inspected only by the playback-start-first readers" — confirmed; and "the media-start-only ones are all <video>/<audio>-scoped" holds for all four (compiler /<(?:video|audio)[^>]*>/gi; parser's composition branch has no offset read; mixer audio[id][src] + video[…][data-has-audio="true"]; validate audio[data-duration]).
  • "data-media-start still works there as a fallback" — correct, via readElementPlaybackStart (media.ts:16-21, Element-typed, called at timeline.ts:415 on the kind: "composition" branch) and Studio's ungated playbackStartValue ?? legacyMediaStartValue (timelineElementHelpers.ts:157-159).
  • "the canonical name Studio writes and normalises to" — playbackStartAttributeForElement returns data-playback-start for every kind === "composition" regardless of what was authored (:182), and compositionInsertion.ts:209 stamps it on inserted hosts. If anything "for new composition hosts" understates it: it normalises existing ones too.

The row now gives the right rule and a reason that survives being checked. That was the item worth the extra round.

Every assertion in the gate now bites

Six mutations at this head, all caught:

Mutation Result
Remove only e.preventDefault() 11 / 1
Remove only e.stopPropagation() 11 / 1
Revert the handler to bare onClick={hasAudio ? toggleSound : togglePlay} 11 / 1
startBoth()a.play() in toggleSound 11 / 1
Drop setMuted(true) from the offscreen teardown 11 / 1
Re-gate the sync effect on reduced 11 / 1

12/12 clean at head. Splitting the nav check into its own test() with the two assert.match calls separated is better than what I proposed — it distinguishes "default not prevented" from "click still bubbles" instead of failing them as one.

Label-in-name: fixed here, and one more instance outside the diff

ReplicaCompare's four names now open with the visible string ("Sound off — …" / "Sound on — …"), so the visible label is contained and it leads, which is what speech input needs. Correct.

Applying the same check across every snippet that sets an aria-label, one more case of the class turns up — not in this PR, and I mention it only because it's the same defect and cheap to see:

  • docs/snippets/docs-video.jsx:346aria-label={Playback speed ${playbackRate} times} on a button whose visible text is {playbackRate}×. At 1.5 the visible label is 1.5× and the accessible name is "Playback speed 1.5 times", so the visible text isn't contained ("×" vs "times"). `${playbackRate}× playback speed` would fix it.
  • Everything else is clean: the other docs-video.jsx controls are icon-only with aria-hidden SVGs, hover-video.jsx is icon-only, and workflow-chooser.jsx:115 opens with route.title, which is the visible text.

Verified clean

  • Rebase. origin/main still an ancestor, 0 behind. Diff since 83db94e7 is 3 files, +18 −5, nothing outside the three items.
  • Docs. Nav 308 / 29 redirects / 0 collisions / dangling / dupes; 1321 internal links / 0 unresolved.
  • Gate. 12/12, every assertion mutation-checked above.

The only thing standing between this and a green board on my side is the oxfmt wrap. Merge still isn't mine, and there's a live changes-request in front of it.

Review by Rames Jusso

The long assert.match line tripped oxfmt --check in CI (Preflight/Format red,
which cascaded regression/player-perf/preview-regression into skip-then-fail). The
pre-commit format hook doesn't cover scripts/*.mjs, so it slipped through. Wrapped
per the formatter. Flagged by Magi and Rames.

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

Round 7 at exact head aa928f399a571fa9b61c67a931db4a97dfb1875d.

The sole round-6 blocker is closed exactly as requested: scripts/check-docs-snippet-motion.test.mjs:170-174 now matches oxfmt's multiline output. I reran bunx oxfmt@0.41.0 --check on the exact-head file and the focused motion suite; formatting is clean and all 12 tests pass. The delta from fed2baf2 is one file, +5/−1, with no implementation change.

GitHub's exact-head board is terminal: 57 success, 3 expected skips, 0 failed/pending. The active main ruleset requires 8 contexts—Semantic PR title, runtime contract, Typecheck, Build, regression, Test, and both Windows jobs—and all 8 report success. The branch is mergeable; BLOCKED is the review gate, not CI or conflicts.

The one still-unresolved Copilot thread is already fixed in source: scripts/changelog-weekly.ts:16,30 imports and reuses CHANGELOG_STYLE_NOTE from set-version.ts; only the thread state is stale.

Audited: the complete fed2baf2...aa928f399 delta; exact-head formatting and 12-test motion gate; active ruleset and every terminal check; unresolved review-thread state. This is cumulative with my rounds 1–6 source/generator/browser review of the Catalog, GSAP, media-offset, reduced-motion, lazy-loading, navigation, and accessibility changes.

Trusting: unchanged portions of the 100-file PR already covered in prior rounds and independent reviews; no new code in those surfaces at this head.

Verdict: APPROVE

Reasoning: The only remaining blocker was a deterministic formatter failure; the exact oxfmt delta landed, local focused verification passes, and every required exact-head context is green.

— Magi

@ukimsanov
ukimsanov merged commit b793e42 into main Aug 5, 2026
60 checks passed
@ukimsanov
ukimsanov deleted the docs/pages-show-not-tell branch August 5, 2026 19:28
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