From 13f6f1a32bb7216a67d0b55729b54fd833aa639f Mon Sep 17 00:00:00 2001 From: Nimrod Kramer <41470823+nimrodkra@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:22:18 +0300 Subject: [PATCH 1/2] feat: randomize mobile feed ad positions via adJitter Add optional `adJitter` to `FeedAdTemplate` so ad positions can vary deterministically per feed while keeping average cadence unchanged. Jitter only activates when the `feed_ad_template` GrowthBook flag sets `adJitter` under the `default` key, so only the mobile/1-column list layout is affected. - Extend FeedAdTemplate with optional adJitter - Pure getAdSlotIndex helper with seeded FNV-1a hash keyed off feedQueryKey - Clamp jitter to floor((adRepeat - 1) / 2) so consecutive ads never overlap or reorder - Unit tests for parity, window bounds, determinism, and clamp Made-with: Cursor --- packages/shared/src/hooks/useFeed.spec.ts | 106 ++++++++++++++++++++++ packages/shared/src/hooks/useFeed.ts | 67 +++++++++++--- packages/shared/src/lib/feed.ts | 1 + 3 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 packages/shared/src/hooks/useFeed.spec.ts diff --git a/packages/shared/src/hooks/useFeed.spec.ts b/packages/shared/src/hooks/useFeed.spec.ts new file mode 100644 index 0000000000..001d7061a9 --- /dev/null +++ b/packages/shared/src/hooks/useFeed.spec.ts @@ -0,0 +1,106 @@ +import { getAdSlotIndex } from './useFeed'; + +describe('getAdSlotIndex', () => { + const seed = '["feed","my-feed"]'; + + it('matches modulo math when adJitter is 0', () => { + const adStart = 2; + const adRepeat = 5; + const positions: number[] = []; + for (let index = 0; index < 40; index += 1) { + const n = getAdSlotIndex({ index, adStart, adRepeat, seed }); + if (n !== undefined) { + positions.push(index); + } + } + expect(positions).toEqual([2, 7, 12, 17, 22, 27, 32, 37]); + }); + + it('returns undefined for indices before the first possible ad slot', () => { + const adStart = 2; + const adRepeat = 5; + const adJitter = 2; + expect( + getAdSlotIndex({ index: -1, adStart, adRepeat, adJitter, seed }), + ).toBeUndefined(); + }); + + it('keeps jittered positions inside the expected window per slot', () => { + const adStart = 2; + const adRepeat = 5; + const adJitter = 2; + const windows = new Map(); + for (let index = 0; index < 60; index += 1) { + const n = getAdSlotIndex({ + index, + adStart, + adRepeat, + adJitter, + seed, + }); + if (n !== undefined) { + const list = windows.get(n) ?? []; + list.push(index); + windows.set(n, list); + } + } + Array.from(windows.entries()).forEach(([n, hits]) => { + expect(hits).toHaveLength(1); + const center = adStart + n * adRepeat; + expect(hits[0]).toBeGreaterThanOrEqual(center - adJitter); + expect(hits[0]).toBeLessThanOrEqual(center + adJitter); + }); + expect(windows.size).toBeGreaterThanOrEqual(10); + }); + + it('is deterministic for the same seed and slot index', () => { + const args = { + adStart: 2, + adRepeat: 5, + adJitter: 2, + seed, + }; + const runA: Array = []; + const runB: Array = []; + for (let index = 0; index < 40; index += 1) { + runA.push(getAdSlotIndex({ index, ...args })); + runB.push(getAdSlotIndex({ index, ...args })); + } + expect(runA).toEqual(runB); + }); + + it('produces different positions for different seeds', () => { + const args = { adStart: 2, adRepeat: 5, adJitter: 2 }; + const collect = (s: string): number[] => { + const hits: number[] = []; + for (let index = 0; index < 60; index += 1) { + if (getAdSlotIndex({ index, ...args, seed: s }) !== undefined) { + hits.push(index); + } + } + return hits; + }; + const seedA = '["feed","user-a"]'; + const seedB = '["feed","user-b"]'; + expect(collect(seedA)).not.toEqual(collect(seedB)); + }); + + it('clamps jitter so consecutive ads never overlap or reorder', () => { + const adStart = 2; + const adRepeat = 5; + const adJitter = 100; + const hits: number[] = []; + for (let index = 0; index < 200; index += 1) { + if ( + getAdSlotIndex({ index, adStart, adRepeat, adJitter, seed }) !== + undefined + ) { + hits.push(index); + } + } + expect(hits.length).toBeGreaterThan(5); + for (let i = 1; i < hits.length; i += 1) { + expect(hits[i]).toBeGreaterThan(hits[i - 1]); + } + }); +}); diff --git a/packages/shared/src/hooks/useFeed.ts b/packages/shared/src/hooks/useFeed.ts index 80864c79b6..5bda5c9f50 100644 --- a/packages/shared/src/hooks/useFeed.ts +++ b/packages/shared/src/hooks/useFeed.ts @@ -150,6 +150,50 @@ export interface UseFeedOptionalParams { onEmptyFeed?: () => void; } +/* eslint-disable no-bitwise -- intentional bitwise ops for FNV-1a hash */ +const hashSeed = (key: string, n: number): number => { + let h = 2166136261 >>> 0; + const s = `${key}:${n}`; + for (let i = 0; i < s.length; i += 1) { + h ^= s.charCodeAt(i); + h = Math.imul(h, 16777619) >>> 0; + } + return h; +}; +/* eslint-enable no-bitwise */ + +export const getAdSlotIndex = ({ + index, + adStart, + adRepeat, + adJitter = 0, + seed, +}: { + index: number; + adStart: number; + adRepeat: number; + adJitter?: number; + seed: string; +}): number | undefined => { + if (adRepeat <= 0) { + return undefined; + } + const safeJitter = Math.max( + 0, + Math.min(adJitter, Math.floor((adRepeat - 1) / 2)), + ); + if (index < adStart - safeJitter) { + return undefined; + } + const n = Math.max(0, Math.round((index - adStart) / adRepeat)); + const offset = + safeJitter === 0 + ? 0 + : (hashSeed(seed, n) % (safeJitter * 2 + 1)) - safeJitter; + const pos = adStart + n * adRepeat + offset; + return pos === index ? n : undefined; +}; + export default function useFeed( feedQueryKey: QueryKey, pageSize: number, @@ -318,21 +362,20 @@ export default function useFeed( adTemplate?.adStart ?? featureFeedAdTemplate.defaultValue.default.adStart; const adRepeat = adTemplate?.adRepeat ?? pageSize + 1; + const adJitter = adTemplate?.adJitter ?? 0; + + const adPage = getAdSlotIndex({ + index, + adStart, + adRepeat, + adJitter, + seed: JSON.stringify(feedQueryKey), + }); - const adIndex = index - adStart; // 0-based index from adStart - - // if adIndex is negative, it means we are not supposed to show ads yet based on adStart - if (adIndex < 0) { - return undefined; - } - const adMatch = adIndex % adRepeat === 0; // should ad be shown at this index based on adRepeat - - if (!adMatch) { + if (adPage === undefined) { return undefined; } - const adPage = adIndex / adRepeat; // page number for ad - if (isLoading) { return createPlaceholderItem(adPage); } @@ -365,8 +408,10 @@ export default function useFeed( isLoading, adTemplate?.adStart, adTemplate?.adRepeat, + adTemplate?.adJitter, adsUpdatedAt, pageSize, + feedQueryKey, ], ); diff --git a/packages/shared/src/lib/feed.ts b/packages/shared/src/lib/feed.ts index b4a97ae582..1ed6547009 100644 --- a/packages/shared/src/lib/feed.ts +++ b/packages/shared/src/lib/feed.ts @@ -274,6 +274,7 @@ export const getFeedName = ( export type FeedAdTemplate = { adStart: number; adRepeat?: number; + adJitter?: number; }; export function usePostLogEvent() { From 1ee1d31ff81f8ddb5bded659a0be3b6e80504b5f Mon Sep 17 00:00:00 2001 From: Nimrod Kramer <41470823+nimrodkra@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:35:54 +0300 Subject: [PATCH 2/2] refactor: address review on feed ad jitter - Move getAdSlotIndex and hashSeed from useFeed.ts to lib/feed.ts since they are pure, React-free utilities. - Clamp first ad to never land before adStart by using one-sided jitter (0..+adJitter) for n=0; subsequent slots keep symmetric jitter. This matches the documented behavior "first ad in posts 3-5" rather than 1-5. - Co-locate spec next to source (lib/feed.spec.ts) and add tests for the adStart lower bound (no jitter), the adRepeat <= 0 guard, and the first-ad lower bound across many seeds. Made-with: Cursor --- packages/shared/src/hooks/useFeed.ts | 45 +----------------- .../useFeed.spec.ts => lib/feed.spec.ts} | 41 ++++++++++++++-- packages/shared/src/lib/feed.ts | 47 +++++++++++++++++++ 3 files changed, 84 insertions(+), 49 deletions(-) rename packages/shared/src/{hooks/useFeed.spec.ts => lib/feed.spec.ts} (70%) diff --git a/packages/shared/src/hooks/useFeed.ts b/packages/shared/src/hooks/useFeed.ts index 5bda5c9f50..997c1137ff 100644 --- a/packages/shared/src/hooks/useFeed.ts +++ b/packages/shared/src/hooks/useFeed.ts @@ -29,6 +29,7 @@ import { usePlusSubscription } from './usePlusSubscription'; import { LogEvent } from '../lib/log'; import { useLogContext } from '../contexts/LogContext'; import type { FeedAdTemplate } from '../lib/feed'; +import { getAdSlotIndex } from '../lib/feed'; import { featureFeedAdTemplate } from '../lib/featureManagement'; import { cloudinaryPostImageCoverPlaceholder } from '../lib/image'; import { AD_PLACEHOLDER_SOURCE_ID } from '../lib/constants'; @@ -150,50 +151,6 @@ export interface UseFeedOptionalParams { onEmptyFeed?: () => void; } -/* eslint-disable no-bitwise -- intentional bitwise ops for FNV-1a hash */ -const hashSeed = (key: string, n: number): number => { - let h = 2166136261 >>> 0; - const s = `${key}:${n}`; - for (let i = 0; i < s.length; i += 1) { - h ^= s.charCodeAt(i); - h = Math.imul(h, 16777619) >>> 0; - } - return h; -}; -/* eslint-enable no-bitwise */ - -export const getAdSlotIndex = ({ - index, - adStart, - adRepeat, - adJitter = 0, - seed, -}: { - index: number; - adStart: number; - adRepeat: number; - adJitter?: number; - seed: string; -}): number | undefined => { - if (adRepeat <= 0) { - return undefined; - } - const safeJitter = Math.max( - 0, - Math.min(adJitter, Math.floor((adRepeat - 1) / 2)), - ); - if (index < adStart - safeJitter) { - return undefined; - } - const n = Math.max(0, Math.round((index - adStart) / adRepeat)); - const offset = - safeJitter === 0 - ? 0 - : (hashSeed(seed, n) % (safeJitter * 2 + 1)) - safeJitter; - const pos = adStart + n * adRepeat + offset; - return pos === index ? n : undefined; -}; - export default function useFeed( feedQueryKey: QueryKey, pageSize: number, diff --git a/packages/shared/src/hooks/useFeed.spec.ts b/packages/shared/src/lib/feed.spec.ts similarity index 70% rename from packages/shared/src/hooks/useFeed.spec.ts rename to packages/shared/src/lib/feed.spec.ts index 001d7061a9..84abb09b52 100644 --- a/packages/shared/src/hooks/useFeed.spec.ts +++ b/packages/shared/src/lib/feed.spec.ts @@ -1,4 +1,4 @@ -import { getAdSlotIndex } from './useFeed'; +import { getAdSlotIndex } from './feed'; describe('getAdSlotIndex', () => { const seed = '["feed","my-feed"]'; @@ -16,15 +16,45 @@ describe('getAdSlotIndex', () => { expect(positions).toEqual([2, 7, 12, 17, 22, 27, 32, 37]); }); - it('returns undefined for indices before the first possible ad slot', () => { + it('returns undefined for indices before adStart (no jitter)', () => { const adStart = 2; const adRepeat = 5; - const adJitter = 2; expect( - getAdSlotIndex({ index: -1, adStart, adRepeat, adJitter, seed }), + getAdSlotIndex({ index: 0, adStart, adRepeat, seed }), + ).toBeUndefined(); + expect( + getAdSlotIndex({ index: 1, adStart, adRepeat, seed }), + ).toBeUndefined(); + }); + + it('returns undefined for non-positive adRepeat', () => { + expect( + getAdSlotIndex({ index: 2, adStart: 2, adRepeat: 0, seed }), ).toBeUndefined(); }); + it('never places the first ad before adStart, even with jitter', () => { + const adStart = 2; + const adRepeat = 5; + const adJitter = 2; + const seeds = Array.from({ length: 50 }, (_, i) => `["feed","user-${i}"]`); + seeds.forEach((s) => { + let firstHit: number | undefined; + for (let index = 0; index < adRepeat; index += 1) { + if ( + getAdSlotIndex({ index, adStart, adRepeat, adJitter, seed: s }) !== + undefined + ) { + firstHit = index; + break; + } + } + expect(firstHit).toBeDefined(); + expect(firstHit).toBeGreaterThanOrEqual(adStart); + expect(firstHit).toBeLessThanOrEqual(adStart + adJitter); + }); + }); + it('keeps jittered positions inside the expected window per slot', () => { const adStart = 2; const adRepeat = 5; @@ -47,7 +77,8 @@ describe('getAdSlotIndex', () => { Array.from(windows.entries()).forEach(([n, hits]) => { expect(hits).toHaveLength(1); const center = adStart + n * adRepeat; - expect(hits[0]).toBeGreaterThanOrEqual(center - adJitter); + const lowerBound = n === 0 ? adStart : center - adJitter; + expect(hits[0]).toBeGreaterThanOrEqual(lowerBound); expect(hits[0]).toBeLessThanOrEqual(center + adJitter); }); expect(windows.size).toBeGreaterThanOrEqual(10); diff --git a/packages/shared/src/lib/feed.ts b/packages/shared/src/lib/feed.ts index 1ed6547009..d51e543943 100644 --- a/packages/shared/src/lib/feed.ts +++ b/packages/shared/src/lib/feed.ts @@ -277,6 +277,53 @@ export type FeedAdTemplate = { adJitter?: number; }; +/* eslint-disable no-bitwise -- intentional bitwise ops for FNV-1a hash */ +const hashSeed = (key: string, n: number): number => { + let h = 2166136261 >>> 0; + const s = `${key}:${n}`; + for (let i = 0; i < s.length; i += 1) { + h ^= s.charCodeAt(i); + h = Math.imul(h, 16777619) >>> 0; + } + return h; +}; +/* eslint-enable no-bitwise */ + +export const getAdSlotIndex = ({ + index, + adStart, + adRepeat, + adJitter = 0, + seed, +}: { + index: number; + adStart: number; + adRepeat: number; + adJitter?: number; + seed: string; +}): number | undefined => { + if (adRepeat <= 0) { + return undefined; + } + if (index < adStart) { + return undefined; + } + const safeJitter = Math.max( + 0, + Math.min(adJitter, Math.floor((adRepeat - 1) / 2)), + ); + const n = Math.max(0, Math.round((index - adStart) / adRepeat)); + // For n === 0, only allow non-negative jitter so the first ad never lands + // before adStart. For n > 0, jitter is symmetric around the cadence. + const isFirst = n === 0; + const range = isFirst ? safeJitter + 1 : safeJitter * 2 + 1; + const baseOffset = isFirst ? 0 : -safeJitter; + const offset = + safeJitter === 0 ? 0 : (hashSeed(seed, n) % range) + baseOffset; + const pos = adStart + n * adRepeat + offset; + return pos === index ? n : undefined; +}; + export function usePostLogEvent() { const { boostedBy } = useFeedCardContext();