From c3ee2852291da5ab440d8d91aad1df60e408d61f Mon Sep 17 00:00:00 2001 From: James Grugett Date: Tue, 28 Apr 2026 17:41:09 -0700 Subject: [PATCH 1/2] Simplify ad response shape --- cli/src/chat.tsx | 9 +- cli/src/components/waiting-room-screen.tsx | 11 +- cli/src/hooks/use-gravity-ad.ts | 142 ++++----------------- web/src/app/api/v1/ads/_post.ts | 46 +++---- web/src/lib/ad-providers/carbon.ts | 2 +- web/src/lib/ad-providers/gravity.ts | 24 +--- web/src/lib/ad-providers/types.ts | 7 +- 7 files changed, 57 insertions(+), 184 deletions(-) diff --git a/cli/src/chat.tsx b/cli/src/chat.tsx index 09727ea6e..a8bae5b03 100644 --- a/cli/src/chat.tsx +++ b/cli/src/chat.tsx @@ -174,7 +174,7 @@ export const Chat = ({ }) const hasSubscription = subscriptionData?.hasSubscription ?? false - const { adData, recordImpression } = useGravityAd({ + const { ads, recordImpression } = useGravityAd({ enabled: IS_FREEBUFF || !hasSubscription, provider: 'gravity', fallbackProvider: 'carbon', @@ -1463,11 +1463,8 @@ export const Chat = ({ /> )} - {adData && (IS_FREEBUFF || getAdsEnabled()) && ( - + {ads && (IS_FREEBUFF || getAdsEnabled()) && ( + )} {reviewMode ? ( diff --git a/cli/src/components/waiting-room-screen.tsx b/cli/src/components/waiting-room-screen.tsx index 7cc0aca4a..9ccba664a 100644 --- a/cli/src/components/waiting-room-screen.tsx +++ b/cli/src/components/waiting-room-screen.tsx @@ -115,7 +115,7 @@ export const WaitingRoomScreen: React.FC = ({ // forceStart bypasses the "wait for first user message" gate inside the hook, // which would otherwise block ads here since no conversation exists yet. // Try Gravity first, then fall back to Carbon when Gravity doesn't fill. - const { adData, recordImpression } = useGravityAd({ + const { ads, recordImpression } = useGravityAd({ enabled: true, forceStart: true, provider: 'gravity', @@ -369,17 +369,14 @@ export const WaitingRoomScreen: React.FC = ({ {/* Ad banner pinned to the bottom, same look-and-feel as in chat. */} - {adData && ( + {ads && ( - + )} {/* Horizontal separator (mirrors chat input divider style) */} - {!adData && ( + {!ads && ( {'─'.repeat(terminalWidth)} diff --git a/cli/src/hooks/use-gravity-ad.ts b/cli/src/hooks/use-gravity-ad.ts index ea6977864..0741eb043 100644 --- a/cli/src/hooks/use-gravity-ad.ts +++ b/cli/src/hooks/use-gravity-ad.ts @@ -9,7 +9,7 @@ import { getAuthToken } from '../utils/auth' import { IS_FREEBUFF } from '../utils/constants' import { logger } from '../utils/logger' -import type { Message} from '@codebuff/sdk'; +import type { Message } from '@codebuff/sdk' const AD_ROTATION_INTERVAL_MS = 60 * 1000 // 60 seconds per ad const MAX_ADS_AFTER_ACTIVITY = 3 // Show up to 3 ads after last activity, then pause fetching new ads @@ -28,8 +28,6 @@ export type AdResponse = { credits?: number // Set after impression is recorded (in cents) } -export type AdVariant = 'banner' | 'choice' - /** * Which upstream ad network to query. The server maps each provider onto the * same normalized response shape, so the rest of the hook is provider-agnostic. @@ -37,43 +35,19 @@ export type AdVariant = 'banner' | 'choice' export type AdProvider = 'gravity' | 'carbon' export type AdSurface = 'waiting_room' -export type AdData = - | { variant: 'banner'; ad: AdResponse } - | { variant: 'choice'; ads: AdResponse[] } - export type GravityAdState = { - ad: AdResponse | null - adData: AdData | null + ads: AdResponse[] | null isLoading: boolean recordImpression: (impUrl: string) => void } // Consolidated controller state for the ad rotation logic type GravityController = { - cache: AdResponse[] - cacheIndex: number choiceCache: AdResponse[][] // Cache of choice ad sets (each entry is 4 ads) choiceCacheIndex: number - variant: AdVariant | null // Assigned variant from backend impressionsFired: Set adsShownSinceActivity: number tickInFlight: boolean - intervalId: ReturnType | null -} - -// Pure helper: add an ad to the cache (if not already present) -function addToCache(ctrl: GravityController, ad: AdResponse): void { - if (ctrl.cache.some((x) => x.impUrl === ad.impUrl)) return - if (ctrl.cache.length >= MAX_AD_CACHE_SIZE) ctrl.cache.shift() - ctrl.cache.push(ad) -} - -// Pure helper: get the next cached ad (cycles through the cache) -function nextFromCache(ctrl: GravityController): AdResponse | null { - if (ctrl.cache.length === 0) return null - const ad = ctrl.cache[ctrl.cacheIndex % ctrl.cache.length]! - ctrl.cacheIndex = (ctrl.cacheIndex + 1) % ctrl.cache.length - return ad } // Pure helper: add a choice ad set to the choice cache @@ -121,8 +95,7 @@ export const useGravityAd = (options?: { const provider: AdProvider = options?.provider ?? 'gravity' const fallbackProvider = options?.fallbackProvider const surface = options?.surface - const [ad, setAd] = useState(null) - const [adData, setAdData] = useState(null) + const [ads, setAds] = useState(null) const [isLoading, setIsLoading] = useState(false) // Check if terminal height is too small to show ads @@ -146,19 +119,15 @@ export const useGravityAd = (options?: { // Single consolidated controller ref const ctrlRef = useRef({ - cache: [], - cacheIndex: 0, choiceCache: [], choiceCacheIndex: 0, - variant: null, impressionsFired: new Set(), adsShownSinceActivity: 0, tickInFlight: false, - intervalId: null, }) // Ref for the tick function (avoids useCallback dependency issues) - const tickRef = useRef<() => void>(() => { }) + const tickRef = useRef<() => void>(() => {}) // Ref to track whether ads should be hidden for use in async code const shouldHideAdsRef = useRef(shouldHideAds) @@ -197,26 +166,12 @@ export const useGravityAd = (options?: { { creditsGranted: data.creditsGranted }, '[ads] Ad impression credits granted', ) - setAd((cur) => - cur?.impUrl === impUrl - ? { ...cur, credits: data.creditsGranted } - : cur, - ) - // Also update credits in adData for choice ads - setAdData((cur) => { + // Also update credits in visible ads + setAds((cur) => { if (!cur) return cur - if (cur.variant === 'choice') { - return { - ...cur, - ads: cur.ads.map((a) => - a.impUrl === impUrl ? { ...a, credits: data.creditsGranted } : a, - ), - } - } - if (cur.variant === 'banner' && cur.ad.impUrl === impUrl) { - return { ...cur, ad: { ...cur.ad, credits: data.creditsGranted } } - } - return cur + return cur.map((a) => + a.impUrl === impUrl ? { ...a, credits: data.creditsGranted } : a, + ) }) } }) @@ -225,23 +180,12 @@ export const useGravityAd = (options?: { }) } - // Show a single banner ad and fire impression - const showAd = (next: AdResponse): void => { - setAd(next) - setAdData({ variant: 'banner', ad: next }) - recordImpressionOnce(next.impUrl) - } - // Show a choice ad set (impressions are fired by the component for visible ads only) const showChoiceAds = (ads: AdResponse[]): void => { - setAd(ads[0] ?? null) // Keep backwards compat for ad field - setAdData({ variant: 'choice', ads }) + setAds(ads) } - type FetchAdResult = - | { variant: 'banner'; ad: AdResponse } - | { variant: 'choice'; ads: AdResponse[] } - | null + type FetchAdResult = { ads: AdResponse[] } | null // Fetch an ad via web API const fetchAd = async (): Promise => { @@ -324,21 +268,15 @@ export const useGravityAd = (options?: { } const data = await response.json() - const variant = data.variant ?? 'banner' - - if ( - variant === 'choice' && - Array.isArray(data.ads) && - data.ads.length > 0 - ) { - return { variant: 'choice', ads: data.ads as AdResponse[] } - } - if (data.ad) { - return { variant: 'banner', ad: data.ad as AdResponse } + if (Array.isArray(data.ads) && data.ads.length > 0) { + return { ads: data.ads as AdResponse[] } } } catch (err) { - logger.error({ err, provider: providerToTry }, '[ads] Failed to fetch ad') + logger.error( + { err, provider: providerToTry }, + '[ads] Failed to fetch ad', + ) } } @@ -363,30 +301,15 @@ export const useGravityAd = (options?: { const result = canFetchNew ? await fetchAd() : null if (result) { - ctrl.variant = result.variant - if (result.variant === 'choice') { - addToChoiceCache(ctrl, result.ads) - ctrl.adsShownSinceActivity += 1 - showChoiceAds(result.ads) - } else { - addToCache(ctrl, result.ad) - ctrl.adsShownSinceActivity += 1 - showAd(result.ad) - } + addToChoiceCache(ctrl, result.ads) + ctrl.adsShownSinceActivity += 1 + showChoiceAds(result.ads) } else { // Fall back to cached ads - if (ctrl.variant === 'choice') { - const cachedSet = nextFromChoiceCache(ctrl) - if (cachedSet) { - ctrl.adsShownSinceActivity += 1 - showChoiceAds(cachedSet) - } - } else { - const next = nextFromCache(ctrl) - if (next) { - ctrl.adsShownSinceActivity += 1 - showAd(next) - } + const cachedSet = nextFromChoiceCache(ctrl) + if (cachedSet) { + ctrl.adsShownSinceActivity += 1 + showChoiceAds(cachedSet) } } } finally { @@ -414,14 +337,8 @@ export const useGravityAd = (options?: { const result = await fetchAd() if (result) { const ctrl = ctrlRef.current - ctrl.variant = result.variant - if (result.variant === 'choice') { - addToChoiceCache(ctrl, result.ads) - showChoiceAds(result.ads) - } else { - addToCache(ctrl, result.ad) - showAd(result.ad) - } + addToChoiceCache(ctrl, result.ads) + showChoiceAds(result.ads) ctrl.adsShownSinceActivity = 1 } setIsLoading(false) @@ -429,19 +346,16 @@ export const useGravityAd = (options?: { // Start interval for rotation (consistent 60s intervals) const id = setInterval(() => tickRef.current(), AD_ROTATION_INTERVAL_MS) - ctrlRef.current.intervalId = id return () => { clearInterval(id) - ctrlRef.current.intervalId = null } }, [shouldStart, shouldHideAds, provider, fallbackProvider, surface]) - // Don't return ad when ads should be hidden + // Don't return ads when ads should be hidden const visible = shouldStart && !shouldHideAds return { - ad: visible ? ad : null, - adData: visible ? adData : null, + ads: visible ? ads : null, isLoading, recordImpression: recordImpressionOnce, } diff --git a/web/src/app/api/v1/ads/_post.ts b/web/src/app/api/v1/ads/_post.ts index a56846b05..c615fc3b1 100644 --- a/web/src/app/api/v1/ads/_post.ts +++ b/web/src/app/api/v1/ads/_post.ts @@ -53,6 +53,13 @@ export type AdsEnv = { CB_ENVIRONMENT: string } +function noAdsResponse(provider: AdProviderId) { + return NextResponse.json( + { ads: [], variant: 'choice', provider }, + { status: 200 }, + ) +} + export async function postAds(params: { req: NextRequest getUserInfoFromApiKey: GetUserInfoFromApiKeyFn @@ -119,13 +126,13 @@ export async function postAds(params: { if (providerId === 'carbon') { if (!serverEnv.CARBON_ZONE_KEY) { logger.warn('[ads] CARBON_ZONE_KEY not configured') - return NextResponse.json({ ad: null, provider: providerId }, { status: 200 }) + return noAdsResponse(providerId) } provider = createCarbonProvider({ zoneKey: serverEnv.CARBON_ZONE_KEY }) } else { if (!serverEnv.GRAVITY_API_KEY) { logger.warn('[ads] GRAVITY_API_KEY not configured') - return NextResponse.json({ ad: null, provider: providerId }, { status: 200 }) + return noAdsResponse(providerId) } provider = createGravityProvider({ apiKey: serverEnv.GRAVITY_API_KEY }) } @@ -146,20 +153,14 @@ export async function postAds(params: { }) if (!result) { - return NextResponse.json( - { ad: null, provider: provider.id }, - { status: 200 }, - ) + return noAdsResponse(provider.id) } - const adsToPersist: NormalizedAd[] = - result.variant === 'choice' ? result.ads : [result.ad] - // Persist served ads so the impression endpoint can validate + fire the // correct pixels. Any DB failure is logged but doesn't block serving. try { await Promise.all( - adsToPersist.map((ad) => + result.ads.map((ad) => db .insert(schema.adImpression) .values({ @@ -184,7 +185,7 @@ export async function postAds(params: { { userId, provider: provider.id, - adCount: adsToPersist.length, + adCount: result.ads.length, error: dbError instanceof Error ? { name: dbError.name, message: dbError.message } @@ -200,25 +201,13 @@ export async function postAds(params: { return rest } - if (result.variant === 'choice') { - logger.info( - { provider: provider.id, variant: 'choice', adCount: result.ads.length }, - '[ads] Fetched choice ads', - ) - return NextResponse.json({ - ads: result.ads.map(toClient), - variant: 'choice', - provider: provider.id, - }) - } - logger.info( - { provider: provider.id, variant: 'banner' }, - '[ads] Fetched banner ad', + { provider: provider.id, variant: 'choice', adCount: result.ads.length }, + '[ads] Fetched choice ads', ) return NextResponse.json({ - ad: toClient(result.ad), - variant: 'banner', + ads: result.ads.map(toClient), + variant: 'choice', provider: provider.id, }) } catch (error) { @@ -235,7 +224,8 @@ export async function postAds(params: { ) return NextResponse.json( { - ad: null, + ads: [], + variant: 'choice', provider: providerId, error: getErrorObject(error), }, diff --git a/web/src/lib/ad-providers/carbon.ts b/web/src/lib/ad-providers/carbon.ts index 64a926436..f4775a00a 100644 --- a/web/src/lib/ad-providers/carbon.ts +++ b/web/src/lib/ad-providers/carbon.ts @@ -164,7 +164,7 @@ export function createCarbonProvider(config: { return null } - return { variant: 'choice', ads } + return { ads } }, } } diff --git a/web/src/lib/ad-providers/gravity.ts b/web/src/lib/ad-providers/gravity.ts index 4ae33b514..e0e8efec4 100644 --- a/web/src/lib/ad-providers/gravity.ts +++ b/web/src/lib/ad-providers/gravity.ts @@ -1,18 +1,14 @@ -import { createHash } from 'crypto' - import { buildArray } from '@codebuff/common/util/array' import type { AdMessage, AdProvider, - AdVariant, FetchAdInput, FetchAdResult, NormalizedAd, } from './types' const GRAVITY_URL = 'https://server.trygravity.ai/api/v1/ad' -const BANNER_PLACEMENT_ID = 'code-assist-ad' const CHOICE_PLACEMENT_IDS = [ 'choice-ad-1', 'choice-ad-2', @@ -50,15 +46,6 @@ function normalize(raw: GravityRawAd): NormalizedAd { } } -/** - * A/B test: deterministically assign a user to the `banner` or `choice` - * variant based on their userId. Stable across requests. - */ -function getGravityVariant(userId: string): AdVariant { - const hash = createHash('sha256').update(`ad-variant:${userId}`).digest() - return hash[0] % 2 === 0 ? 'banner' : 'choice' -} - /** * Extract the content from the last tag in a string. * The CLI wraps raw user text in that tag; if no tag is found, returns the @@ -111,16 +98,12 @@ export function createGravityProvider(config: { apiKey: string }): AdProvider { fetch, } = input - const variant = - input.surface === 'waiting_room' ? 'choice' : getGravityVariant(userId) const filteredMessages = prepareGravityMessages(messages) const placementIds = input.surface === 'waiting_room' ? WAITING_ROOM_PLACEMENT_IDS - : variant === 'choice' - ? CHOICE_PLACEMENT_IDS - : [BANNER_PLACEMENT_ID] + : CHOICE_PLACEMENT_IDS const placements = placementIds.map((id) => ({ placement: 'below_response', @@ -192,10 +175,7 @@ export function createGravityProvider(config: { apiKey: string }): AdProvider { return null } - if (variant === 'choice') { - return { variant: 'choice', ads: ads.map(normalize) } - } - return { variant: 'banner', ad: normalize(ads[0]) } + return { ads: ads.map(normalize) } }, } } diff --git a/web/src/lib/ad-providers/types.ts b/web/src/lib/ad-providers/types.ts index fb3284e2a..ced439e8f 100644 --- a/web/src/lib/ad-providers/types.ts +++ b/web/src/lib/ad-providers/types.ts @@ -8,8 +8,6 @@ import type { Logger } from '@codebuff/common/types/contracts/logger' */ export type AdProviderId = 'gravity' | 'carbon' -export type AdVariant = 'banner' | 'choice' - /** * Normalized ad shape returned by every provider. The CLI renders against * this shape; provider modules are responsible for mapping their upstream @@ -62,10 +60,7 @@ export type FetchAdInput = { fetch: typeof globalThis.fetch } -export type FetchAdResult = - | { variant: 'banner'; ad: NormalizedAd } - | { variant: 'choice'; ads: NormalizedAd[] } - | null +export type FetchAdResult = { ads: NormalizedAd[] } | null export type AdProvider = { id: AdProviderId From c8ca33021f78d8040a0e829d3e914a143fc211a5 Mon Sep 17 00:00:00 2001 From: James Grugett Date: Tue, 28 Apr 2026 17:54:32 -0700 Subject: [PATCH 2/2] Address ad review simplifications --- cli/src/hooks/use-gravity-ad.ts | 11 +++-------- web/src/app/api/v1/ads/_post.ts | 11 +++-------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/cli/src/hooks/use-gravity-ad.ts b/cli/src/hooks/use-gravity-ad.ts index 0741eb043..0a7f2e9e6 100644 --- a/cli/src/hooks/use-gravity-ad.ts +++ b/cli/src/hooks/use-gravity-ad.ts @@ -180,11 +180,6 @@ export const useGravityAd = (options?: { }) } - // Show a choice ad set (impressions are fired by the component for visible ads only) - const showChoiceAds = (ads: AdResponse[]): void => { - setAds(ads) - } - type FetchAdResult = { ads: AdResponse[] } | null // Fetch an ad via web API @@ -303,13 +298,13 @@ export const useGravityAd = (options?: { if (result) { addToChoiceCache(ctrl, result.ads) ctrl.adsShownSinceActivity += 1 - showChoiceAds(result.ads) + setAds(result.ads) } else { // Fall back to cached ads const cachedSet = nextFromChoiceCache(ctrl) if (cachedSet) { ctrl.adsShownSinceActivity += 1 - showChoiceAds(cachedSet) + setAds(cachedSet) } } } finally { @@ -338,7 +333,7 @@ export const useGravityAd = (options?: { if (result) { const ctrl = ctrlRef.current addToChoiceCache(ctrl, result.ads) - showChoiceAds(result.ads) + setAds(result.ads) ctrl.adsShownSinceActivity = 1 } setIsLoading(false) diff --git a/web/src/app/api/v1/ads/_post.ts b/web/src/app/api/v1/ads/_post.ts index c615fc3b1..370f11622 100644 --- a/web/src/app/api/v1/ads/_post.ts +++ b/web/src/app/api/v1/ads/_post.ts @@ -54,10 +54,7 @@ export type AdsEnv = { } function noAdsResponse(provider: AdProviderId) { - return NextResponse.json( - { ads: [], variant: 'choice', provider }, - { status: 200 }, - ) + return NextResponse.json({ ads: [], provider }, { status: 200 }) } export async function postAds(params: { @@ -202,12 +199,11 @@ export async function postAds(params: { } logger.info( - { provider: provider.id, variant: 'choice', adCount: result.ads.length }, - '[ads] Fetched choice ads', + { provider: provider.id, adCount: result.ads.length }, + '[ads] Fetched ads', ) return NextResponse.json({ ads: result.ads.map(toClient), - variant: 'choice', provider: provider.id, }) } catch (error) { @@ -225,7 +221,6 @@ export async function postAds(params: { return NextResponse.json( { ads: [], - variant: 'choice', provider: providerId, error: getErrorObject(error), },