feat: randomize mobile feed ad positions via adJitter

[nimrodkra]

Summary

Adds an optional adJitter knob to the existing feed_ad_template GrowthBook flag so mobile ad positions (first ad + gaps) vary deterministically per feed. Desktop is untouched because the flag is already per-breakpoint and adJitter is only set under the default key at rollout time.

Today mobile injects an ad at post 3 and then at a fixed cadence, which feels predictable. With adJitter: 2 on default, the first ad lands in posts 3–5 and each subsequent gap varies between adRepeat ± 2, keeping average frequency unchanged.

Changes

• FeedAdTemplate gains optional adJitter?: number.
• New pure helper getAdSlotIndex in packages/shared/src/lib/feed.ts with a seeded FNV-1a hash keyed off feedQueryKey. Positions are stable across re-renders / pagination but differ per feed / user.
• getAd in useFeed uses the helper; deps updated to include adTemplate?.adJitter and feedQueryKey.
• Jitter for n = 0 is one-sided (0..+adJitter) so the first ad can never land before adStart; subsequent slots use symmetric jitter (-adJitter..+adJitter).
• Jitter is clamped to floor((adRepeat - 1) / 2) so consecutive ads can never overlap or reorder, even with misconfigured flag values.
• Unit tests (8/8) cover jitter=0 parity with today's modulo math, adStart lower bound with and without jitter, adRepeat <= 0 guard, window bounds per slot, determinism, seed variance, and overlap clamp.

Rollout

Only affects mobile when the feed_ad_template payload sets adJitter under the default key. Example:

{
  "default":  { "adStart": 2, "adRepeat": 5, "adJitter": 2 },
  "tablet":   { "adStart": 2, "adRepeat": 5 },
  "laptop":   { "adStart": 2, "adRepeat": 5 },
  "laptopL":  { "adStart": 2, "adRepeat": 5 },
  "laptopXL": { "adStart": 2, "adRepeat": 5 },
  "desktop":  { "adStart": 2, "adRepeat": 5 }
}

Absent adJitter means today's behavior — safe to merge before flipping the flag.

Test plan

• pnpm --filter shared lint
• pnpm exec jest src/lib/feed.spec.ts (8/8 pass)
• Strict typecheck on changed files
• A/B on default breakpoint (adJitter: 0 vs adJitter: 2) to measure CTR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
daily-webapp	Ready	Preview	Apr 21, 2026 7:40am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
storybook	Ignored		Apr 21, 2026 7:40am

[nimrodkra]

@claude please review

[claude]

Claude finished @nimrodkra's task in 3m 27s —— View job

---

Code Review

• Gather context and understand the request
• Read changed files
• Analyze code for bugs, correctness, and best practices
• Post review feedback

---

Overall this is a clean, well-reasoned implementation. The FNV-1a hash is a solid choice for a lightweight deterministic seed, and the clamping invariant (safeJitter ≤ floor((adRepeat-1)/2)) is the key insight that makes the algorithm provably correct (no two adjacent slots can ever collide). The tests cover the main scenarios well. A few things worth addressing:

---

Issues

1. First ad can appear at index 0 — contradicts the PR description

With { adStart: 2, adRepeat: 5, adJitter: 2 }, the early-exit guard is:

// useFeed.ts:185
if (index < adStart - safeJitter) {  // 2 - 2 = 0 → only guards index < 0
  return undefined;
}

For index = 0, n = 0, and if hashSeed(seed, 0) % 5 = 0 (i.e. offset = -2), then pos = 2 + 0 - 2 = 0, so an ad lands as the very first feed item. The PR description says "first ad lands in posts 3–5" (1-indexed), but the code allows posts 1–5. If that lower bound being 0 is intentional, the description should say so; if not, you'd want to clamp: const pos = Math.max(adStart, adStart + n * adRepeat + offset) for n = 0, or raise adStart in the payload to 4 so it jitters to 2–6.

Fix this →

---

2. Pure utility function lives in a hook file

getAdSlotIndex and hashSeed are pure, React-free functions. Per the project architecture (lib/ = utilities, hooks = React logic), they belong in packages/shared/src/lib/feed.ts alongside FeedAdTemplate. Exporting a pure function from useFeed.ts creates a slightly awkward import path for callers and tests.

Fix this →

---

Minor / Nits

Test coverage gap — The "returns undefined before first possible slot" test only asserts on index: -1:

// useFeed.spec.ts:22-26
it('returns undefined for indices before the first possible ad slot', () => {
  expect(
    getAdSlotIndex({ index: -1, adStart, adRepeat, adJitter, seed }),
  ).toBeUndefined();
});

A more meaningful test would also cover e.g. index: 1, adStart: 2, adRepeat: 5, adJitter: 0 (no jitter, before adStart) to confirm the basic no-jitter guard works, since -1 is an impossible feed index anyway.

---

adRepeat <= 0 edge case — The function returns undefined correctly when adRepeat <= 0, but there's no test for it. Worth a one-liner since it's an explicit guard.

---

Seed stability note — JSON.stringify(feedQueryKey) is stable for the same logical feed/user, but object-valued keys (e.g. variables embedded in the key) could in theory change serialization order across environments. The current usage appears to be simple string/number arrays so this is fine in practice — just worth a comment if the key shape ever changes.

---

What's solid

• The clamping formula Math.min(adJitter, Math.floor((adRepeat - 1) / 2)) is the correct invariant: windows never overlap and slots can never reorder. ✅
• Math.imul(...) >>> 0 keeps the hash unsigned so % is always non-negative. ✅
• deps array in getAd correctly includes adTemplate?.adJitter and feedQueryKey. ✅
• The no-jitter path is tested to be identical to the old modulo math. ✅
• Absent adJitter in the GrowthBook payload defaults to 0 (line 365), so this is safely backward-compatible. ✅