feat(registry): add avatar promo and Slack notification templates - #3279
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
The two additions are unusually well-packaged: every manifest path resolves, the avatar MP4/audio durations match the declared clip durations exactly, both font families carry OFL text, and the generated registry/index/vector metadata includes both IDs. I also like that the moving belt is one linear timeline rather than a clock-driven loop.\n\nReview scope\n- Audited: both composition sources, both manifests, every referenced asset/license, media metadata, generated registry/index membership, the exact CLI checks, transition sampling, and rendered frames.\n- Trusting: the generated MDX and vector binary byte content, backed by the green docs/catalog-artifact/preview CI rather than hand-reviewing generated payloads.\n\n### [P1] The promoted Slack render contains visible missing-glyph boxes\n\nThe declared emoji face is only a local macOS font (), while the content includes emoji in the notifications (lines 425-441).\n\nI rendered the submitted block with the exact published CLI:\n\n\n\nThe renderer reports . At 3.3s the praying-hands character renders as a narrow vertical box after “Friday?”, and at 8.7s the grimacing-face character renders as the same box after “launch video”. These are visible in the shipped frames; the green result does not catch the failed local font. Because the internal promotion packages this exact HTML, the defect would propagate there too.\n\nPlease either remove/replace those emoji or ship a redistributable deterministic glyph asset/font, then visually verify the affected arrival frames in the Linux renderer.\n\nEverything else I exercised is clean: registry-item lint is 2/2, both checks have zero lint/runtime errors, all sampled contrast checks pass, the intentional belt/phone entrance overflow is clipped as designed, and all OSS CI is green at this SHA.\n\nVerdict: REQUEST CHANGES \nReasoning: The catalog/package plumbing is correct, but two user-visible notification strings render broken in the actual renderer environment.\n\n— Magi
Replaced immediately with a correctly formatted review; verdict and finding unchanged.
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
The two additions are unusually well-packaged: every manifest path resolves, the avatar MP4/audio durations match the declared clip durations exactly, both font families carry OFL text, and the generated registry/index/vector metadata includes both IDs. The moving belt is also one deterministic linear timeline rather than a clock-driven loop.
Review scope
- Audited: both composition sources, both manifests, every referenced asset/license, media metadata, generated registry/index membership, the exact
0.7.109CLI checks, transition sampling, and rendered frames. - Trusting: the generated MDX and vector binary byte content, backed by the green docs/catalog-artifact/preview CI rather than hand-reviewing generated payloads.
[P1] The promoted Slack render contains visible missing-glyph boxes
The declared emoji face is only a local macOS font (slack-notification-ad.html:29-30), while the content includes emoji in the notifications (lines 425-441).
I rendered the submitted block with the exact published CLI:
npx hyperframes@0.7.109 snapshot ... --at 3.3,5.8,8.7,9.5
The renderer reports Fonts FAILED: Apple Color Emoji. At 3.3s the praying-hands character renders as a narrow vertical NO GLYPH box after “Friday?”, and at 8.7s the grimacing-face character renders as the same box after “launch video”. These are visible in the shipped frames; the green hyperframes check result does not catch the failed local font. Because the internal promotion packages this exact HTML, the defect would propagate there too.
Please either remove/replace those emoji or ship a redistributable deterministic glyph asset/font, then visually verify the affected arrival frames in the Linux renderer.
Everything else I exercised is clean: registry-item lint is 2/2, both checks have zero lint/runtime errors, all sampled contrast checks pass, the intentional belt/phone entrance overflow is clipped as designed, and all OSS CI is green at this SHA.
Verdict: REQUEST CHANGES
Reasoning: The catalog/package plumbing is correct, but two user-visible notification strings render broken in the actual renderer environment.
— Magi
|
Addressed in 0bdb959. Removed the macOS-only Apple Color Emoji face and all three emoji characters from notification copy, so the block now depends only on the bundled OFL Inter files. Registry lint passes 375/375, HyperFrames static lint is clean aside from the existing monolithic-file warning, and the refreshed Catalog Previews renderer job is green at this head. Please re-review. |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Incremental re-review at exact head 0bdb9598264c0ae0dddda4a68ae9bb37f07d2c88.
The runtime fix itself is correct: registry/blocks/slack-notification-ad/slack-notification-ad.html:25-45 removes the local-only Apple emoji face, and :419-439 removes all three unsupported glyphs. The refreshed catalog renderer job is green.
blocker — regenerate the tracked catalog page
docs/catalog/blocks/slack-notification-ad.mdx:68-86 still publishes the removed Apple Color Emoji face, and :466-480 still publishes all three emoji in the Source accordion. That accordion is user-facing copyable source, generated directly from the registry primary file by scripts/generate-catalog-pages.ts:793-806, so the catalog currently documents the exact broken version this commit fixed.
Please regenerate the tracked catalog page from the corrected registry source. The green Catalog Previews workflow does not catch this sync gap: it renders the registry item and uploads preview artifacts, but does not regenerate or compare the MDX source.
Verdict: REQUEST CHANGES
Reasoning: The rendered/installable registry block is fixed, but a second shipped representation of the same template remains stale and still exposes the broken source.
— Magi
|
Addressed in |
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Incremental re-review at exact head 961246075c445971f276c82992ee85534e38a41c.
The only delta since my review at 0bdb9598 is the regenerated docs/catalog/blocks/slack-notification-ad.mdx (+4/-9). I compared the MDX Source fence against registry/blocks/slack-notification-ad/slack-notification-ad.html: they are byte-for-byte identical at this head.
The generated artifact now uses only "NotificationUI", sans-serif at docs/catalog/blocks/slack-notification-ad.mdx:81, and its notification copy at :461-475 matches the glyph-safe runtime source. A current-head grep across both files has zero hits for Apple Color Emoji|🙏|😅|😬.
The prior full audit remains valid. Exact-head docs validation, catalog rendering/preflight, registry/search coverage, CodeQL, Mintlify, and WIP checks are green; there are zero unresolved review threads.
Audited: this one-file generated delta, source/generated byte parity, prohibited-font/glyph sweep, current review state, and exact-head CI; carried forward the prior full audit of both compositions, manifests, assets/licenses, metadata, registry/index membership, transitions, and rendered frames.
Trusting: unchanged generated media/vector payloads already covered by the prior audit and green render checks.
Verdict: APPROVE
Reasoning: The final shipped MDX representation now matches the glyph-safe runtime source, closing the only remaining blocker without changing previously audited behavior.
— Magi
Superseded immediately by the same exact-head approval with the complete review body.
2 participants
Summary:
Verification: