feat(media-use): resolve official brand logos via a four-tier cascade#2061
Conversation
Third-party brand logos (the meeting's 'credibility signals lost' gap) had no acquisition path: capture only grabs the product's own site assets, and HeyGen asset search returns generic look-alike icons for brand queries (0/3 in testing — an X-in-a-circle for LinkedIn). Workers could only fake a mark or drop it. New resolve type 'logo', four tiers verified by a 54-brand stress test (100% cascade hit across dev tools / big tech / non-tech / CN brands): - svgl — official full-color vector SVGs + wordmark variants (40/54 first-hits); search is substring-based, so entities pass through alias normalization (nextjs → 'next.js', aws → 'amazon web services') - simple-icons (pinned CDN build) — official monochrome glyphs; catches the long tail (nike, visa, toyota, wechat, bytedance) - github org avatar — known-org map only; a brand name is not a GitHub login, guessing risks same-named personal accounts - domain favicon (DuckDuckGo ip3) — small-raster last resort; sub-500B responses are DDG's placeholder and rejected; frozen with a low_res provenance flag (chip-size use only) logo joins the icon/image equivalence group (typesMatch) and the images/ subdir, so entity cache hits interop with figma-imported marks. A total miss falls through resolve's normal failure path — no special casing. HeyGen search stays the icon provider; it is deliberately absent from the logo cascade. Docs: media-use gap/types/providers tables + example; the five workflow banners now cover logos (catalog claim kept for media, 'from their official sources' added for logos); product-launch story-design and motion-graphics logo-reveal point at the new type; catalog surfaces (CLAUDE.md / README / docs) updated in lockstep. Verified: 19 unit tests + coverage row green; live smoke across all four tiers (linkedin→svgl, nike→simple-icons, heygen→github.avatar, amazon→favicon) plus a fabricated brand exiting 1 on the default miss path. oxlint + oxfmt clean.
…owlist svgl / simple-icons / github.avatar / favicon.ddg join the sanctioned list — the logo cascade added in the previous commit. Full lib suite 95/95 green.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
miga-heygen
left a comment
There was a problem hiding this comment.
Review: LGTM ✅
Clean, well-scoped feature addition. The four-tier cascade (svgl → simple-icons → GitHub org avatar → domain favicon) is well-reasoned with appropriate guardrails at each level.
SSOT audit — clean
entityFrom— single source for entity extraction from intent/flag. ✓norm()— single normalizer used consistently across all lookup maps (SVGL_ALIASES,SIMPLE_ICON_SLUGS,GITHUB_ORGS,FAVICON_DOMAINS). ✓- Cascade ordering — each tier has a distinct host and distinct match criteria. No duplicated decisions. ✓
typesMatchinterop —logocorrectly maps toicon/imagetype match. ✓
Verified
- Tier guardrails: Exact
titleMatcheson svgl (no fuzzy matches), known-org-only on GitHub (GITHUB_ORGSmap), 500-byte threshold on favicon (catches placeholder/error pages). Correct. - Network error handling: Consistent pattern across all four tiers — network failure returns
null(exits function, lets cascade continue to next tier), data mismatch doescontinue(tries next alias query on same host). Correct — network down means same host won't work for another query. - Multi-word entities:
entityFrom("Red Bull logo")→"red bull"works correctly throughnorm()for all lookup maps. svgl handles multi-word queries natively. - Fallback domain construction:
faviconDomainForusesnorm(entity) + ".com"with explicit overrides inFAVICON_DOMAINSfor non-obvious mappings (coca-cola, etc.). Reasonable best-effort for a last-resort tier. - Documentation: Updates consistent across CLAUDE.md, README, docs, SKILL.md, and all workflow skill banners.
- Test coverage: All pure helper functions covered (
entityFrom,titleMatches,svglQueriesFor,simpleIconSlugsFor,githubOrgFor,faviconDomainFor). Plus the 54-brand stress test mentioned in PR description.
Nits (non-blocking)
-
No unit tests for async search functions — The four search functions (
svglSearch,simpleIconsSearch,githubAvatarSearch,faviconSearch) aren't tested in the unit suite. The pure-function coverage is solid and the stress test exists externally, but even a mocked-fetch test verifying the return shape would strengthen the contract. -
faviconSearchfull GET for byte check — Does a full GET and reads the body to checkbyteLength. Fine for tiny favicon payloads, andHEADwouldn't give reliableContent-Lengthfrom DDG's ip3.
Ship it.
— Miga
jrusso1020
left a comment
There was a problem hiding this comment.
Additive review — COMMENT (external author; stamp = James's call)
Audited: skills/media-use/scripts/lib/logo-provider.mjs end-to-end; registry.mjs:59-68 cascade wiring + runProviders orchestrator at registry.mjs:137-148; match.mjs type-equivalence update; the test files (logo-provider.test.mjs, coverage.test.mjs); the 21-file catalog sync (SKILL.md updates + README/CLAUDE.md/docs/guides).
Trusting: Miga's arch-clean audit (single-normalizer, single-entity-extraction, no cross-provider decision duplication) — those hold on inspection; not duplicating.
Additive findings
-
(cascade orchestrator correctness — verified)
registry.mjs:139-146:for (const p of providers) { ... const res = await fn(intent, ctx); if (res) return res; } return null;. Each provider returnsnullon every miss condition (network throw, HTTP not-ok, no-hit-after-all-aliases, placeholder rejected), so the cascade correctly falls through — no silent swallow of a miss into a next-tier bypass.runProvidersis documented as the "unit-testable core of the cascade" but doesn't actually have a unit test in this PR. Adding one for logo (mock 4 providers returning[null, null, {hit}, null]and assert 3rd wins) would pin Home's Lens 1 invariant as a real CI gate — currently it's inspectable but not gated. -
(test coverage gap — worth flagging clearly) Miga notes "the 54-brand stress test gives good confidence on the async paths" — but that stress-test is a manual live-smoke result cited in the linked heygenverse artifact, not part of the CI suite. The 6 shipped unit tests (
logo-provider.test.mjs) cover pure helpers (entityFrom, titleMatches, svglQueriesFor, simpleIconSlugsFor, githubOrgFor, faviconDomainFor) — solid, well-scoped, but zero coverage of: (a) cascade fallthrough on tier-above miss, (b) network-error → null propagation, (c) svglitemsnon-array → skip-to-next-query, (d) favicon <500B → null (DDG placeholder rejection), (e) simple-icons HEAD 404 → next slug attempt. Miga's "mocked-fetch unit test" suggestion is right — the "100% cascade hit" is currently asserted by author testing, not guarded by CI. Not blocking, but the invariant regression risk is real once the CDN URLs / API shapes drift. -
(favicon double-fetch — minor cost)
faviconSearchatlogo-provider.mjs:189-201downloads the full bytes just for the size-check (bytes = (await res.arrayBuffer()).byteLength), then returns only a URL descriptor. The subsequent freeze step inresolve.mjs(viafreezeUrl) re-downloads. For favicon-tier hits, that's 2 network round-trips per logo. Small but non-zero. Options: (a) accept it (favicon is the last-resort tail, low frequency), (b) push the buffer through the descriptor so the freeze step can skip its own fetch. Not blocking. -
(Home's Lens 2 spot-check — mostly punted to
freeze.mjs) The{url, ext, source, metadata}descriptors returned by each tier are consumed byresolve.mjs → freezeUrl(imported atresolve.mjs:10). I did not fully auditfreeze.mjsfor path-collision handling on same-name resolves (linkedin.svgfrom two projects), fetch-failure post-descriptor (URL 200 at HEAD, 500 at GET, or DNS resurrection mid-run), or non-image response bodies (svgl route serving HTML on error). Those failure modes are infreeze.mjs, not touched by this PR — but the new tier providers do widen the surface (the favicon tier can trigger writes on IP3's placeholder had the byte-check ever regressed). Worth a follow-up audit offreeze.mjs's error handling in a separate pass; not a blocker on this PR. -
(nit / cascade doc)
logo-provider.mjs:2-22describes the tiers with hit counts (40/54, 11/54, 2/54, 1/54 = 54 total exactly). That's a nice header, but it will drift the moment the alias maps orGITHUB_ORGSchange. Consider adding "hit-count last-verified: 2026-07 stress test" so a future reader knows it's a snapshot, not a live invariant. -
(strength) The
!== nullreturn contract is honored consistently: every tier's failure path isreturn null— noundefined, nothrow, no error-object return.runProviders' truthy check works uniformly against this. Rare to see this shape stay this clean across 4 different tier implementations. -
(strength) Lookalike rejection via
titleMatchesis a real correctness win:assert.ok(!titleMatches("Slackware", "slack"))atlogo-provider.test.mjs:19is exactly the kind of substring-search failure mode that would silently ship the wrong logo. Well-caught by the exact-title-check step after the svgl substring search.
Verdict
COMMENT — cascade architecture is clean and Home's Lens 1 (each tier fallback triggers on tier-above miss, no silent swallow) is verified in the orchestrator loop; the write-path lens (Lens 2) mostly punts to freeze.mjs (not touched here, low risk); the test coverage lens (Lens 3) has an explicit gap between pure-helper unit tests and the "100% cascade hit" live-smoke claim — worth a follow-up mock-network cascade test rather than treating the manual smoke as CI-safe. CI is green on bdfd8fb5. Stamp = @jrusso1020's call (external author WaterrrForever; requester Miao is allowlisted but not a trusted-stamper, per delegation rule).
— Rames Jusso
…favicon tier Review follow-ups (miga-heygen, jrusso1020 on #2061): - Eight mocked-network tests pin what the manual 54-brand stress test only asserted: descriptor shape, alias retry (svgl non-array payload → next query, simple-icons 404 → next slug), network-error → null fallthrough, the sub-500B placeholder rejection, github's no-guessing (zero fetches for unmapped entities), and the real cascade order landing tier by tier under a mocked network. - faviconSearch now hands its verified bytes over as a local file, so the freeze step copies instead of re-downloading — one round-trip, and the size check is authoritative over what gets frozen. - The header's hit counts are labeled as a stress-test snapshot, not a live invariant. Full lib suite 103/103; live smoke re-verified (amazon → favicon.ddg, frozen .ico).
|
Thanks @miga-heygen @jrusso1020 — the substantive points are all in ( Fixed
Tracked as a follow-up
Live smoke re-verified after the favicon change (amazon → favicon.ddg, frozen |
| // gets frozen and the favicon tier costs one network round-trip, not two. | ||
| const bytes = body.byteLength; | ||
| const tmp = join(mkdtempSync(join(tmpdir(), "media-use-logo-")), `${domain}.ico`); | ||
| writeFileSync(tmp, body); |
Esper Labs flagged it directly: third-party logos (e.g. LinkedIn) are not reliably included in videos, and the credibility signal is lost. Today there is no acquisition path at all — capture only grabs the product's own site assets, and HeyGen asset search returns generic look-alike icons for brand queries (0/3 in testing: an X-in-a-circle for "LinkedIn logo", a chat bubble for "Slack"). Workers can only fake a mark with CSS or drop the mention.
What
A new resolve type —
resolve --type logo --entity <brand>— that returns the brand's official mark as a frozen local file, through a four-tier cascade. Verified by a 54-brand stress test (dev tools / big tech / non-tech / CN brands): 100% cascade hit, with each tier catching what the one above misses.Changes
lib/logo-provider.mjs(new) — the four tiers:nextjs→ "next.js",aws→ "amazon web services"), and every hit is exact-title-checked ("slack" must never match "Slackware").low_resprovenance flag for chip-size use only (1/54: amazon).registry.mjs— thelogocascade, all four tiers free and network-marked (--local-onlyleaves only the cache rungs). HeyGen asset search is deliberately absent from this cascade; it stays theiconprovider.logojoins the icon/image equivalence group (typesMatch) and theimages/subdir, so entity cache hits interop with figma-imported brand marks; default ext.svg.Guardrails
no provider could resolve logo: "…", exit 1) — no special casing.Tests / verification
linkedin→ svgl (.svg),nike→ simple-icons (.svg),heygen→ github.avatar (.png),amazon→ favicon (.ico); a fabricated brand exits 1 on the default miss path.