docs(registry,skills): surface code-highlight 0-based indexing and opacity-reveal sweep guidance#2418
Conversation
…acity-reveal sweep guidance From the 2026-07-14 CLI feedback digest (skills-owner action): a user building code teaching videos hit two authoring gaps. 1. code-highlight's `line` is intentionally zero-based (`line: 1` = second displayed line) but the warning lived only in pr-to-video's code-vocabulary reference — nowhere an author actually touches the value. Call it out at the block-use sites: the `__BLOCK` declaration itself, the registry-item description, and the motion-graphics catalog map. 2. Opacity-only code-typing tripped `sweep_static` for that user, who worked around it with a slow host y-drift. The sweep fingerprint does include per-element opacity, so document the actual trap (a reveal that settles before the sampled window, then holds a static frame) and the idiomatic fixes (spread the reveal / keep a blinking caret alive) in the check reference — and pin the fingerprint's opacity sensitivity with a regression test covering both the visibility-floor crossing and a mid-fade value change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
92c76b3 to
5db35a7
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Review — #2418 docs(registry,skills): surface code-highlight 0-based indexing and opacity-reveal sweep guidance
Verdict: LGTM — clean docs + regression test, well-placed.
SSOT check
The 0-based line note lands on three consumption surfaces, each serving a different reader:
| Surface | Reader |
|---|---|
Inline comment in code-highlight.html |
Author editing the HTML block config |
registry-item.json description |
Agent discovering blocks via the registry |
catalog-map.md code/code-reveal row |
Motion-graphics workflow routing to blocks |
Same fact, three surfaces — not a duplicated decision. The behavior is owned by the code itself; the docs surface it where people encounter it. The code-scroll contrast ("unlike code-scroll, whose target is 1-based") in the registry-item description is helpful — prevents the obvious copy-paste mistake between the two blocks.
sweep_static opacity guidance
The new paragraph in lint-validate-inspect.md is well-placed — it's the canonical check reference. The guidance correctly explains the failure mode (reveal settles before the sampled window → every sample sees the static settled state) and recommends the real fix (spread the reveal or keep a continuously animated element alive) instead of the hack (slow position drift). The "blinking caret is idiomatic for code typing" recommendation is practical and specific.
Regression test
The opacity-reveal fixture is well-designed:
- Getter-based mock (
get opacity() { return charOpacity; }) cleanly simulates the animation - Three states tested:
hidden(opacity 0, below 0.2 floor),fading(opacity 0.5, mid-reveal),revealed(opacity 1, settled) - Asserts BOTH the visibility-floor crossing (
fading !== hidden) AND mid-range opacity sensitivity (revealed !== fading) - Pins the two behaviors that must hold for opacity-only reveals to count as motion
No blockers. Ship it.
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 5db35a7e. Layered on Miga's LGTM — I traced the source claims + verified the regression test's fingerprint sensitivity, and add one docs-drift nit.
Small, sharp docs + regression PR. The three surfaces the PR touches for the code-highlight 0-based note are exactly where an author actually changes the value; the sweep_static opacity guidance names the real failure mode and the real fix; the regression pins fingerprint opacity sensitivity so a future refactor that drops opacity from the signature fails loud.
Blockers
(none)
Concerns
(none)
Nits
🟡 docs/catalog/blocks/code-highlight.mdx will drift from registry-item.json until next docs regen. scripts/generate-catalog-pages.ts writes the mdx description: frontmatter + H1-body paragraph + docs/public/catalog-index.json entry from manifest.description. Base state was in sync (mdx and registry-item share the exact pre-PR description); this PR updates the registry-item but not the generated mdx, so /catalog/blocks/code-highlight on the docs site will keep the old shorter description until someone runs npx tsx scripts/generate-catalog-pages.ts and commits. Small — nobody edits the mdx by hand, and Mintlify pre-build (per the script's docstring intent) picks it up on next docs deploy — but if you're already touching the description surface, a one-liner bun scripts/generate-catalog-pages.ts && git add docs/ closes the loop for git-tracked search hits. (Aside: docs/public/catalog-index.json on origin/main is missing the code-highlight entry entirely — a pre-existing drift, not this PR's problem, but same generator-run would resolve both.)
Green notes
🟢 0-based claim verified against source. registry/blocks/code-highlight/code-highlight.html:683-684 — var idx = Math.max(0, Math.min(lines.length - 1, targetIdx)); var lr = lines[idx].getBoundingClientRect(); on a querySelectorAll(".line") NodeList → spec.line = 1 picks lines[1], i.e. the second .line element. ✓ The inline __BLOCK comment + registry-item + catalog-map wording all match.
🟢 1-based counter-claim for code-scroll verified. registry/blocks/code-scroll/code-scroll.html:1491 — var idx = Math.max(0, Math.min(lines.length - 1, targetLine - 1)); — the targetLine - 1 conversion is the tell that callers pass 1-based; demo default line: 12 (line 1349) is consistent. The "don't carry a value between the two blocks without re-checking" warning in the code-highlight inline comment is doing real work.
🟢 Fingerprint DOES include per-element opacity. packages/cli/src/commands/layout-audit.browser.js:1470-1479 — elements = Array.from(root.querySelectorAll("*")).filter(isVisibleElement) then parts.map emits ${rect.left},${rect.top},${rect.width},${rect.height},${opacity} where opacity = round(opacityChain(element)) (round = 2-decimal precision at line 26). Docstring at line 1429-1434 explicitly calls this out ("of every visible element's box + opacity, in DOM order"). ✓
🟢 0.2 visibility floor claim verified. isVisibleElement at line 196 — opacityFloor == null ? opacityChain(element) < 0.2 : hasOpacityBelow(element, opacityFloor) — default (no floor arg) uses 0.2, and collectLayoutGeometry at line 1470 calls isVisibleElement(element) without arguments, so 0.2 is the effective floor for the fingerprint. charOpacity = "0" fixture correctly drops #char out of the signature; "0.5" re-admits it; "1" changes the opacity value while it stays in. ✓
🟢 Regression test is a real pin, not a re-assertion. defensive_bug_pinning shape — the three collected strings differ in distinguishable ways: hidden has no #char part, fading has one with the "0.5" opacity slot, revealed has one with the "1" slot. A future refactor that (a) dropped opacity from the per-element part, (b) raised the visibility floor above 0.5, or (c) collapsed opacity precision into a broader bucket would fail this test. Correct regression surface. ✓
🟢 sweep_static guidance names the real trap. The check reference now correctly reads "reveal that completes early and then holds a static frame for the rest of the duration: every sample lands on the settled state and the run fails." That matches the sample-grid + string-equality shape at checkPipeline.ts (fingerprint compared across seeked samples, #U10). Author-facing fix ("spread the reveal across the timeline or keep one continuously animated element alive") over the previous position-drift hack. ✓
🟢 SSOT extension without duplication. The 0-based note lands on the inline __BLOCK comment, registry-item.json, and catalog-map.md — each is where a different reader will actually be when they need it (author editing the HTML, cataloger browsing, motion-graphics skill consumer). skills/pr-to-video/references/code-vocabulary.md already had the definitive index-semantics table; the PR body explicitly notes it "already documents it, but nobody pastes values from there" — correct diagnosis of surface placement, not doc-sprawl.
🟢 CI green at 5db35a7e — full CI (Format/Build/Lint/Fallow/Typecheck/Test/Producer unit+integration/Skills/Runtime contract/Studio smoke/CLI shims/Preview parity/Windows/Player perf/regression/preview-regression/CodeQL) all pass. Regression-shards skipped by change-detection (no engine code touched — correct).
What I didn't verify
- Whether the Mintlify pre-build actually runs
generate-catalog-pages.tsin prod docs deploys (no.github/workflows/*reference to the script found — worth confirming with docs infra before relying on auto-regen).
Verdict framing
Clean PR — docs land where authors touch, sweep_static guidance names the actual failure mode not just a symptom, and the regression pins a semantic (opacity-in-fingerprint) that would be tempting to "simplify" out later. LGTM from my side; the mdx-drift nit is worth a one-line follow-up but not a blocker.
…tem description Only the code-highlight page is committed: a full generate-catalog-pages run also surfaces ~34 blocks missing from the git-tracked catalog index (pre-existing drift on main), which belongs in its own chore PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Closed the mdx-regen nit in de8b1cb: Scoped deliberately to the one page: a full |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
R1 (5db35a7e) → R2 (de8b1cbd) delta: single commit docs(catalog): regenerate code-highlight page from updated registry-item description — docs/catalog/blocks/code-highlight.mdx +2/-2. Reviewed at de8b1cbd.
R1 nit status
🟢 mdx drift closed. Both the frontmatter description: and the H1 body paragraph now include the 0-based note, mirroring registry-item.json character-for-character. Docs site's /catalog/blocks/code-highlight will render the full 0-based-vs-code-scroll caveat on next build. ✓
🟢 Scope kept clean. Only the drifted mdx got touched; the pre-existing docs/public/catalog-index.json gap (code-highlight entry absent from origin/main's manifest — I noted this as a separate observation in R1) is left for a broader catalog regen task. Correct call — that gap predates this PR and shouldn't have blocked this one.
Verdict framing
R1 nit resolved; the rest of the PR (0-based source claim, 1-based code-scroll counter-claim, opacity-in-fingerprint, 0.2 visibility floor, regression-test red-first shape) was already verified at R1. LGTM stands.
jrusso1020
left a comment
There was a problem hiding this comment.
Approved on James's go — verified ready. Rames-D's LGTM stands at the current head de8b1cb (its R1 mdx-drift nit is resolved: the 0-based line note now mirrors registry-item.json in both the frontmatter and the H1), Miga LGTM'd, CI green (42 checks), no open changes-requested. The only delta since the last LGTM was a benign +2/-2 docs regen of the catalog page. Docs + registry + a regression test — low-risk. — Rames Jusso
Summary
code-highlight's zero-basedlineat the places an author actually touches it: an inline comment on the__BLOCKdeclaration, the registry-item description, and the motion-graphics catalog map (pr-to-video's code-vocabulary reference already documents it, but nobody pastes values from there)sweep_statictrap for opacity-only reveals in the check reference: the fingerprint does include per-element opacity, so a reveal counts as motion while in flight — the failure mode is a reveal that settles before the sampled window and then holds a static frame; recommend spreading the reveal or keeping a blinking caret alive instead of bolting on a position driftlayout-audit.browser.test.ts): both the 0.2 visibility-floor crossing and a mid-fade opacity change must alter the signatureContext
Skills-owner action from the 2026-07-14 CLI feedback digest (report): a user building two bash teaching videos hit an off-by-one on
code-highlight(line: Nhighlights gutter N+1 — intentional zero-based design, confirmed by triage) and worked around asweep_staticmisfire on opacity-only code typing with a slow host y-drift hack. Both are authoring-surface gaps, not engine bugs.Verification
layout-audit.browser.test.ts: 65 tests passed, including the new opacity-reveal fixture