feat(monetization): sponsor strip experiment (footer_logo) - #6605
feat(monetization): sponsor strip experiment (footer_logo)#6605rebelchris wants to merge 16 commits into
Conversation
A logo wall docked under the main feeds — one static gold slot, four rotating premium slots and a community row that fills whatever the row has left — with the breaking-news ticker stacked underneath it. While the strip is carrying those headlines, the feed drops its Happening Now card. Impressions, IAB viewable impressions, clicks and a new `air time` duration event are logged per logo, keyed by tier and slot. The ad source is a fixture until the `footer_logo` placement exists on the ad server. Behind `sponsor_strip`, default off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rebelchris
left a comment
rebelchris
left a comment
There was a problem hiding this comment.
Summary
A large, well-tested feature, but as pushed it does not ship default-off — the committed flag default turns the strip on for every eligible reader on merge, and the ad source it renders from is an unguarded fixture of real third-party logos. Those two together are the blocker; the rest is non-blocking.
Blocking
lib/featureManagement.ts:273— flag default isenabled: true.sponsorStrip/mockSponsorStripAds.ts— fixture is not environment-gated, unlikemockFeedHighlights.- CI — the
daily-webappVercel deployment is red on this commit; the preview URL in the description can't be verified until it's green.
Non-blocking
- Comment volume against the repo's code-style rule (
AGENTS.md: comments explain why, and are rare — no multi-line preambles above a component, hook or constant). A sizeable share of the added lines is prose that will drift as the code moves; the same reasoning is already in this PR description, where it stays accurate. AdPlacement.SponsorStriphas no branch inresolveAdFetchOptions/fetchAdByPlacement(noted in the description). Since the enum value is now live, a future caller falls through to the feed endpoint silently — either add the branch or keep the value out until the placement exists.- Verification I could not do from the diff: the
sticky bottom-0dock inside the extension new-tab shell, and the two out-of-tree consumers of--sponsor-strip-height(ScrollToTopButton,FeedbackWidget) with the property unset on every other surface. Worth a screenshot each on webapp + extension at tablet and laptop widths.
Reviewed by AI.
| export const featureSponsorStrip = new Feature<SponsorStripConfig>( | ||
| 'sponsor_strip', | ||
| { | ||
| enabled: true, |
There was a problem hiding this comment.
Blocking: the default is enabled: true, three lines under a comment that says it must stay false, and against the root AGENTS.md rule ("NEVER default an experiment flag to true").
useSponsorStrip returns isEligible && config.enabled, and useConditionalFeature falls back to this committed default whenever GrowthBook has no value for sponsor_strip — so on merge every non-Plus, tablet-and-up reader on a hero-eligible feed gets the strip, and loses the Happening Now card via disableHighlightItems. There is no rollback short of a deploy. The PR description says "default off", so I read this as an unintended leftover from local testing.
The specs don't catch it: useSponsorStrip.spec.tsx mocks useConditionalFeature with enabled: true, so the real default is never exercised. Set it to false and add a case asserting the committed default is off.
Reviewed by AI.
| wallSponsor('Sentry', 'sentry', 512 / 113, SponsorTier.Community), | ||
| ]; | ||
|
|
||
| export const fetchSponsorStripAds = async (): Promise<unknown> => |
There was a problem hiding this comment.
Blocking: this fixture has no environment guard. mockFeedHighlights puts isDevelopment inside the module precisely so no query param can put a fixture in front of a real reader — the ad source needs the same treatment, and needs it more.
Combined with the flag default above, merging renders real third-party marks (Datadog, PostHog, ClickHouse, Sentry, JetBrains, …), hotlinked from business.daily.dev, in a row labelled "Made possible by", to production readers — companies presented as paying sponsors of daily.dev who have not bought the placement, with outbound ?utm_source=sponsor_strip links. That's a brand and legal exposure, not just a data-quality one, and it also feeds real impression/viewable impression/air time/click events for invented gen_ids into the warehouse.
Even with the flag correctly off, the fixture and its links ship in the bundle. Suggest fetchSponsorStripAds returns [] unless isDevelopment, and that the wall marks become neutral placeholder shapes rather than identifiable brands until the footer_logo placement is live.
Reviewed by AI.
| globalThis.removeEventListener('statechange', onLifecycle); | ||
| logEventEnd(key); | ||
| }; | ||
| }, [genId, slotIndex, logEvent, logEventStart, logEventEnd]); |
There was a problem hiding this comment.
Non-blocking: logEvent/logEventStart/logEventEnd are in the dep array, so any re-memoization of the log context value re-runs this effect — which ends the open air-time event and logs a second impression for a logo that never left its slot. sponsor is already held in a ref for exactly this reason; the three callbacks are the same kind of dependency. Worth putting them behind a ref too so the effect keys only on genId/slotIndex.
Reviewed by AI.
| const { data } = useQuery({ | ||
| ...majorHeadlinesQueryOptions({ first: HEADLINE_LIMIT }), | ||
| enabled, | ||
| refetchInterval: ONE_MINUTE, |
There was a problem hiding this comment.
Non-blocking: refetchInterval: ONE_MINUTE means every eligible reader polls majorHeadlines once a minute for the whole time a main feed is open, on top of the highlight items the feed request already carries. Against a 24-hour freshness window a minute buys very little — what's the freshness requirement here? A few minutes reads identically and cuts the request volume by the same factor.
Reviewed by AI.
| }: SponsorStripProps): ReactElement | null => { | ||
| const { gold, premium, community, wallRef } = useSponsorStripAds({ | ||
| enabled: true, | ||
| config, |
There was a problem hiding this comment.
Non-blocking: enabled: true is hardcoded at the only call site, so the enabled prop on useSponsorStripAds (and the enabled it forwards into useRotatingSlots) can never be false — the strip is already gated by sponsorStrip.isEnabled in MainFeedLayout before this mounts. Dead parameter; dropping it removes a branch from the rotation hook too.
Reviewed by AI.
The default was committed as `enabled: true`, which would have shipped the experiment to everyone on merge with no rollback short of a deploy. On in development and off in production instead, the way `cores`, `quest_offers` and `post_boost_ads` are — so a local session still gets the strip while GrowthBook owns the ramp. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Turnover is per page load rather than on a timer: the decks are dealt once on mount, so the row a reader sees holds for as long as the page does and the next load deals a different set. That drops the rotation intervals, and with them the whole config payload — the flag is a boolean like `cores`, which also removes the chance of a mistyped interval reaching `setInterval`. Slot scarcity is unchanged and now stated where it lives: one gold, four premium, community takes whatever the row has left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`feedGutter` collapses to `px-0` inside the v2 floating card, because the feed's inset there comes from the grid being narrower than the frame rather than from padding. The dock spans the frame edge to edge, so it inherited nothing and its first logo sat against the sidebar. It carries the same values `feedGutter` uses outside the frame instead, which is where the cards start in both layouts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the fictional Quantile placeholder in the paid slot, per the design mock-up. The mark is the official four-colour Google Cloud logo, reusing the exact asset the Explore ad-card mock-up already established in this repo rather than sourcing a second copy of it. Its white backing plate is dropped: that plate exists to round-crop the mark into a favicon, and on the strip it would sit as a white tile against the dark ground. The slot keeps its two-ink design — the mark holds its brand colour in both themes while the wordmark flips — which is the case a single flat file cannot serve and the reason the wire contract takes a themed pair. The wordmark is set in the fixture's own stack rather than Google Sans; a licensed face cannot be inlined, and it is the part a real creative replaces anyway. Still a fixture behind the experiment flag. Nothing here says the slot has been sold, and "Made possible by" claims more than a display campaign does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ticker filtered `majorHeadlines` to a 24 hour window client-side, so on any day the feed went quiet the row emptied and the dock silently dropped from two rows to one. That is not rare: checked against the live API, all twelve headlines returned were 36 to 92 hours old and every one was filtered out, leaving the row gone with nothing to tell the reader why. The development-only fixture underneath papered over exactly this on one environment while production had the same hole. The window was never right for this surface. The row stands in for the feed's Happening Now card, which applies no age filter at all — it renders whatever the backend serves — and neither does /highlights. The gate was copied from the post-page sidebar widget, the one surface of the three that has it. Deciding what still counts as a major headline is the backend's call; a second opinion held only by this row just emptied it. So the filter goes, and the fixture with it — the API already returns newest first and every row renders its own relative timestamp. Stays on the light `majorHeadlines` query the widget uses (4.4KB) rather than the /highlights document, which carries post bodies at 147KB for the four fields this row needs. Also refreshes the `DOCK_GUTTER` note: main dropped `px-4` from `feedGutter`, so the two constants no longer match below tablet, where the dock does not render anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…roll `disableHighlightItems` was read off the headlines having arrived, and the headlines are their own round trip that lands after the feed has painted. So the feed rendered its Happening Now card, the query came back, the flag flipped, and the card was pulled out of the middle of the feed with everything below it jumping up. The old 24 hour window hid this: headlines were usually filtered to nothing, so the flip mostly never happened. Serving them unfiltered made it fire on every load. The strip stands in for the card whenever it is up, so that is what the decision reads now. `useStripHeadlines` reports whether its query has answered, and the card is given back only when it settles with nothing at all — breaking news must never fall out of the product entirely just because the experiment is on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The sponsor row and the ticker are two independent round trips into a dock pinned to the bottom of the viewport. The ads resolve first, so the paid row painted alone against the viewport floor, and when the headlines landed a second row appeared underneath it and shoved the logos upward — a visible jump on the row somebody paid for, on every load. Each row now holds its own height open until its own query answers, and collapses only once that query has answered with nothing to show. The empty states are unchanged: no fill still means no sponsor row, no headlines still means no ticker, and neither still means no dock. `useSponsorStripAds` reports whether the ad query has settled, and `useSponsorStripFeed` the same for the headlines. The ticker's impression log is now conditional, because the row is mounted empty while it waits and an impression logged then reported a ticker of nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`sticky bottom-0` reaches the viewport bottom only while its containing block extends past that line. Inside the v2 floating card the block stops 14px short — `laptop:my-3` on the wrapper plus `laptop:p-0.5` on the frame — so the dock had two resting places: 14px high whenever the card's own bottom was what was on screen, and flush when it was not. On first paint the card is `min-h-[calc(100vh-1.5rem)]` and the page is not yet scrollable, so the dock loaded high and dropped once the feed filled. The same jump was visible scrolling to the end of the feed. Pinning to the frame's own inset makes both positions one position, so the dock rests on the card's bottom edge and stays there. Keyed to the frame's class rather than to `isV2`, the way `feedGutter` is, because the flag resolves after mount; outside the frame there is no inset and the offset stays zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pinning the dock to the frame's inset stopped it jumping but parked it 14px above the window, which is not where a dock belongs. It now sits flush and stays there. `sticky bottom-0` reaches the viewport bottom only while its containing block extends past that line, and `laptop:overflow-clip` on the v2 frame means a sticky child can never paint into the frame's gutter. So the frame gives the gutter up instead: where it holds a dock it drops its bottom margin, padding, border and corner rounding, and the dock becomes its bottom edge. Everywhere else the frame is unchanged. Matched with a `has-[.feed-dock]` variant rather than a prop, because the frame is two components above the dock's mount point, and written as a literal because Tailwind scans source text and generates nothing for an interpolated class name. Verified the four rules land inside the laptop media query. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…a dock The frame's `min-h` subtracts the chrome above it plus its own margins, so it finishes exactly at the bottom of the window. Dropping its bottom margin for the dock left that sum 0.75rem too large: the frame stopped 12px short, and a `sticky bottom-0` dock rests at its container's end rather than the window's, so the dock sat 12px high for as long as the feed was too short to make the page scrollable, then dropped when it wasn't. Both values lose the margin that is no longer there, which puts the frame's bottom edge back on the window and the dock flush in either state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four things from review. Alignment: `DOCK_GUTTER` was `feedGutter`'s values copied by hand, and had already drifted — main dropped `px-4` from `feedGutter`, and more importantly `feedGutter` collapses to nothing inside the v2 frame while the copy kept `px-10`, so the dock's contents sat 40px in from the cards above them. The copy is gone; the dock uses `feedGutter` itself, plus the same width clamp the grid uses on wide screens, now shared as `feedWidth` rather than spelled out again. Only the contents are held to those edges — the border and ground still span the dock, so it reads as one bar under a column of cards. Hover: the gold mark's scale-up on hover is gone. Gold sizing: the mark was being optically normalised along with the wall. That normalising trades height for width to stop a dozen unrelated marks fighting, which punishes exactly the wide mark-and-wordmark lockup a paid slot supplies — it came out at 18px, shorter than the tallest silhouette beside it. It is drawn at a flat 28px now and the width follows. The wordmark also moves off Verdana, which is far wider than the real one. Ticker: scrolls horizontally instead of only clipping, so the headlines under the fade can be reached. Scrollbar hidden; the fade already says the row continues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gold slot carried the cloud-symbol lockup, drawn from the devicon mark the Explore ad-card mockup uses, with "Google Cloud" set beside it in a font stack standing in for Google Sans. Google no longer leads with the symbol: cloud.google.com serves gstatic.com/cgc/google-cloud-logo-fullcolor.svg — the four-ink Google wordmark followed by "Cloud", no symbol, in their refreshed palette (#3186FF, #FC413D, #FFBE00, #00AF57) rather than the classic four. That file is the artwork now, so the paid slot shows the mark the advertiser actually publishes instead of an approximation of it. "Cloud" ships as #212226 — near-black, and it all but disappears against the dark feed — so it stays the one ink taken off their file and flipped with the ground, which is what the light/dark pair in the wire contract is for. Nothing else about the artwork changes. GOLD_HEIGHT is the slot's box height, and the swap changes what fills it: a symbol lockup spends most of that box on the symbol, an all-wordmark lockup spends it on letterforms. At 28 the type came out roughly twice the size of the old one, so the box drops to the wall's own ceiling — in colour and half again as wide as a wall slot, it still leads the row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`feedGutter` is where the feed's column starts in the legacy layout, and the dock has used it since the hand-copied `DOCK_GUTTER` went away. Inside the v2 floating card it is zero — the card is already the frame — but the grid then takes a `laptop:p-6` inset of its own so the cards sit off the frame's rounded corners. So in v2 the cards start 24px further in than the gutter does, and the dock's "Made possible by" and "Breaking news" sat a full inset to the left of the column they belong to. Measured on the preview by adding the frame class and then the grid's own inset to a live feed: card left 44 → 68 while the dock's label stayed at 44. That 24px is the whole bug, and it is why this looked correct in v1 — there is no inset there for the gutter to miss. `feedFrameInsetX` carries it, next to `feedGutter` and `feedWidth`, so the three things that decide where the feed's edges are sit together rather than one of them being rediscovered per consumer. Horizontal only: the grid's vertical half is the frame's breathing room, and chrome outside the grid sets its own height. It rides its own element in both rows rather than joining the gutter and the width clamp on one. All three set horizontal padding, and stacked on a single element which one wins is left to the order Tailwind emits them in. Nested, they compose: the gutter finds the frame, the inset finds the cards inside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rebelchris
left a comment
rebelchris
left a comment
There was a problem hiding this comment.
Summary
Re-review of the last 15 commits. The flag default is fixed and CI is green, so the first blocker from the previous round is closed. The second one is not — the ad fixture still has no environment guard, and the gold slot now carries a real company's brand lockup, which makes shipping it a larger problem than it was a week ago. Two blocking comments inline; the rest are non-blocking repeats.
Closed since the last review
featureSponsorStripnow defaults toisDevelopment, matchingcores/quest_offers/post_boost_ads.isDevelopmentisNODE_ENV === 'development', so a production build cannot enrol anyone from the committed default. Good.- CI and the
daily-webapppreview deployment are green ona2e1b0b. - The air-time effect's dependency array is fine after all —
useLogContextreturns auseMemo(..., []), sologEvent/logEventStart/logEventEndare stable for the component's lifetime and the effect cannot re-run and double-log. Withdrawing that comment.
Blocking
mockSponsorStripAds.ts:151— the fixture is still ungated, so ramping the flag ships fabricated sponsorships.mockSponsorStripAds.ts:62— a third party's logo reproduced as inline path data with one ink recoloured.
Non-blocking (all previously raised, still open)
useStripHeadlines.ts:38—refetchInterval: ONE_MINUTEagainst a multi-hour freshness window.SponsorStrip.tsx:172—enabled: trueis a dead parameter onuseSponsorStripAds.AdPlacement.SponsorStripstill has no branch inresolveAdFetchOptions/fetchAdByPlacement, so a future caller falls through to the feed endpoint silently.- Comment density against root
AGENTS.md("match its comment density … the reasoning behind a fix goes in the commit message, not a comment above the code"). This has grown rather than shrunk:mockSponsorStripAds.tsopens with a 26-line preamble and the new lockup constants carry three more prose blocks. The same reasoning is in the PR description, where it stays accurate as the code moves.
Still not verifiable from the diff
The has-[.feed-dock] compensations in MainLayout are a four-value contract between a class name in sponsorStripOffset.ts and literal Tailwind variants two components up, with min-h-[calc(...)] values that have to stay in step with the chrome above the frame. They are correctly scoped — nothing changes with the strip absent — but nothing can verify them either. Worth a screenshot each of: the dock at laptop and tablet width in the v2 frame, the dock in the extension new-tab shell, and ScrollToTopButton + FeedbackWidget lifting and settling back.
Reviewed by AI.
| wallSponsor('Sentry', 'sentry', 512 / 113, SponsorTier.Community), | ||
| ]; | ||
|
|
||
| export const fetchSponsorStripAds = async (): Promise<unknown> => |
There was a problem hiding this comment.
Blocking (repeat). fetchSponsorStripAds still returns MOCK_SPONSOR_STRIP_ADS unconditionally. mockFeedHighlights in this same PR puts isDevelopment inside the module so no query param can put a fixture in front of a real reader; the ad source needs the same guard, and needs it more.
The flag default being isDevelopment closes the merge risk, not the risk this comment is about. The whole point of sponsor_strip being a GrowthBook flag is that someone ramps it — and the moment they do, on any percentage, production readers get a row labelled "Made possible by" holding a dozen named third-party marks that have not bought the placement, with rel="sponsored" outbound links carrying ?utm_source=sponsor_strip, plus real impression / viewable impression / air time / click events keyed to invented gen_ids landing in the warehouse next to real ad reporting.
The module comment says "this stays behind the experiment flag until the slot is actually sold", but nothing in the code enforces that — it relies on whoever ramps the flag having read this file. Make it structural: return [] unless isDevelopment. Then the flag can be ramped safely, the dock's empty-row path gets exercised by the ramp, and the fixture becomes what the ad server replaces rather than something that has to be remembered.
Reviewed by AI.
| * symbol lockup is an older mark they no longer lead with, and the inks here | ||
| * are their refreshed palette, brighter than the classic four. | ||
| */ | ||
| const GOOGLE_WORDMARK = [ |
There was a problem hiding this comment.
Blocking. This is new since the last review and it is the part I would not merge in any form.
The gold slot — the one the row presents as paid, at full height, in its own inks, behind a "Made possible by" label — now names a specific real company, in a public repository, with its wordmark hand-transcribed into eight <path> constants and one ink deliberately changed (CLOUD_WORD flipped from #212226 to the theme ground). Three separate problems, any one of which is enough:
- Commercial. Naming a real, identifiable company in the paid slot of a sponsorship strip, in a public repo, reads as a statement about a commercial relationship. Design mock-ups are internal; this file is not. Whether or not any conversation exists with that company, the repo should not be where it is implied.
- Trademark. Most brand-permission terms — theirs included — allow the mark only unaltered, from the files they publish. Re-drawing the lockup and recolouring one word to suit our dark theme is exactly the alteration those terms exclude. The same applies more weakly to the wall marks, which are hotlinked off
business.daily.devinto a context that labels them sponsors of this placement. - Reviewability (per the repo's own bar for opaque generated constants). Nobody reviewing this diff can tell what those path strings draw. There is no regeneration step, no source file, and no check that would notice if one path were wrong or drifted from what the vendor now serves — and if it renders wrong, it renders wrong as somebody's logo.
For judging weight, spacing and optical sizing — which is what the comment says the artwork is for — neutral placeholder shapes work identically: a coloured block and a word at the measured logo_ratio exercise the same code paths, the same masking, and the same theme pair, without naming anyone. If a real lockup genuinely has to be there to review the design, it belongs in a screenshot on this PR, not in main.
Reviewed by AI.
2 participants
A sponsored logo wall docked under the main feeds, with the breaking-news ticker stacked under it. Design comes from the mock-up in #6490 — this is the "breaking news" concept of it, wired to a flag and to real tracking.
Behind
sponsor_strip, a boolean defaulting toisDevelopment— on locally, off in production, the waycores,quest_offersandpost_boost_adsare. GrowthBook owns the ramp.Slots
Turnover is per page load: the decks are dealt once on mount, so the row holds for as long as the page does and the next load deals a different set — no advertiser is permanently first, or permanently the one a narrow window drops. Nothing rotates on a timer.
Tracking
Four signals per logo, per slot, all through one builder so the placement can be cut by tier and slot:
impressionad.pixel[](viewport-gated, as elsewhere)viewable impressionuseViewability(MRC/IAB 50% for 1s) +viewed=truepixel re-fireclickuseAdClickUrl+rel="sponsored"air timelogEventStart/logEventEnd, so it lands withevent_duration= how long one logo held one slotEvery event carries
extra: { placement: 'footer_logo', tier, slot_index, gen_id }.@ido —
air timeis a new event name and needs to be known downstream; it's the samelogEventStart/logEventEndduration mechanism feed-ad impressions already use.Happening Now
The strip is where breaking news goes while it's up, so the feed drops its Happening Now card — but only when the strip is actually carrying headlines. Strip up with no fresh headline, Plus, ads disabled, or below tablet: the card stays. The user's own
highlightsPlacementsetting is never written, so the card returns when the experiment ends.Not final
mockSponsorStripAds.ts), in the exact wire shape the schema parses, so swapping it is one call:GET /v1/a/footer_logo?count=<n>plus the consent params inlib/ads.ts. NoteAdPlacement.SponsorStriphas no case inresolveAdFetchOptions/fetchAdByPlacementyet — today it would fall through to the feed endpoint, so that switch needs a branch when the placement lands.gitlab.svgon business.daily.dev is a base64 WebP wrapped in an<svg>and masks to a solid block, so it's left out of the fixture. Real creatives are expected to carry a themed light/dark pair.mockFeedHighlights.tsinjects a Happening Now card and the ticker's headlines locally (development only). Delete both mocks when the experiment is decided.Testing
pnpm --filter shared test(2827 pass, 59 in the strip),pnpm --filter webapp test,pnpm --filter extension test, strict changed-file typecheck, lint. Rotation rules, tier partitioning, optical sizing, creative parsing, the gate, the suppression rule, and all four tracking signals have specs.To see it locally: force
sponsor_stripon and load a feed with?mockHighlights=1to get the card for comparison with the flag off.🤖 Generated with Claude Code
Preview domain
https://feat-sponsor-strip.preview.app.daily.dev