Skip to content

[codex] Add Carbon fallback for CLI ads #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jahooma merged 1 commit into from
Apr 24, 2026
+64 −42
Merged

[codex] Add Carbon fallback for CLI ads #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cli/src/chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@ export const Chat = ({
})
const hasSubscription = subscriptionData?.hasSubscription ?? false

const { adData, recordImpression } = useGravityAd({ enabled: IS_FREEBUFF || !hasSubscription })
const { adData, recordImpression } = useGravityAd({
enabled: IS_FREEBUFF || !hasSubscription,
provider: 'gravity',
fallbackProvider: 'carbon',
})

// Set initial mode from CLI flag on mount
useEffect(() => {
Expand Down
5 changes: 3 additions & 2 deletions cli/src/components/waiting-room-screen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,12 @@ export const WaitingRoomScreen: React.FC<WaitingRoomScreenProps> = ({
// Always enable ads in the waiting room — this is where monetization lives.
// forceStart bypasses the "wait for first user message" gate inside the hook,
// which would otherwise block ads here since no conversation exists yet.
// Uses Carbon (BuySellAds); in-chat ads still use the Gravity default.
// Try Gravity first, then fall back to Carbon when Gravity doesn't fill.
const { adData, recordImpression } = useGravityAd({
enabled: true,
forceStart: true,
provider: 'carbon',
provider: 'gravity',
fallbackProvider: 'carbon',
})

useFreebuffCtrlCExit()
Expand Down
95 changes: 56 additions & 39 deletions cli/src/hooks/use-gravity-ad.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,15 @@ export const useGravityAd = (options?: {
/** Skip the "wait for first user message" gate. Used by the freebuff
* waiting room, which has no conversation but still needs ads. */
forceStart?: boolean
/** Which ad network to query. Defaults to Gravity. */
/** Primary ad network to query. Defaults to Gravity. */
provider?: AdProvider
/** Backup ad network to try when the primary returns no fill or errors. */
fallbackProvider?: AdProvider
}): GravityAdState => {
const enabled = options?.enabled ?? true
const forceStart = options?.forceStart ?? false
const provider: AdProvider = options?.provider ?? 'gravity'
const fallbackProvider = options?.fallbackProvider
const [ad, setAd] = useState<AdResponse | null>(null)
const [adData, setAdData] = useState<AdData | null>(null)
const [isLoading, setIsLoading] = useState(false)
Expand Down Expand Up @@ -278,49 +281,63 @@ export const useGravityAd = (options?: {
}
}

try {
const response = await fetch(`${WEBSITE_URL}/api/v1/ads`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${authToken}`,
},
body: JSON.stringify({
provider,
messages: adMessages,
sessionId: useChatStore.getState().chatSessionId,
device: getDeviceInfo(),
// Carbon requires a real browser-ish useragent for targeting/fraud
// detection. Gravity ignores it. We source one centrally so every
// provider that needs it sees the same value.
userAgent: getAdUserAgent(),
}),
})
const providersToTry =
fallbackProvider && fallbackProvider !== provider
? [provider, fallbackProvider]
: [provider]
Comment on lines +284 to +287
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Simplify providersToTry construction

The dedup check (fallbackProvider !== provider) can be collapsed using Set, which also handles the undefined case more naturally without the extra boolean guard:

Suggested change
const providersToTry =
fallbackProvider && fallbackProvider !== provider
? [provider, fallbackProvider]
: [provider]
const providersToTry = [...new Set([provider, fallbackProvider].filter((p): p is AdProvider => p != null))]

This removes the nested ternary and makes the intent ("unique, defined providers in priority order") immediately clear.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: cli/src/hooks/use-gravity-ad.ts
Line: 284-287

Comment:
**Simplify `providersToTry` construction**

The dedup check (`fallbackProvider !== provider`) can be collapsed using `Set`, which also handles the `undefined` case more naturally without the extra boolean guard:

```suggestion
    const providersToTry = [...new Set([provider, fallbackProvider].filter((p): p is AdProvider => p != null))]
```

This removes the nested ternary and makes the intent ("unique, defined providers in priority order") immediately clear.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


if (!response.ok) {
logger.warn(
{ provider, status: response.status, response: await response.json() },
'[ads] Web API returned error',
)
return null
}
for (const providerToTry of providersToTry) {
try {
const response = await fetch(`${WEBSITE_URL}/api/v1/ads`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${authToken}`,
},
body: JSON.stringify({
provider: providerToTry,
messages: adMessages,
sessionId: useChatStore.getState().chatSessionId,
device: getDeviceInfo(),
// Carbon requires a real browser-ish useragent for targeting/fraud
// detection. Gravity ignores it. We source one centrally so every
// provider that needs it sees the same value.
userAgent: getAdUserAgent(),
}),
})

const data = await response.json()
const variant = data.variant ?? 'banner'
if (!response.ok) {
logger.warn(
{
provider: providerToTry,
status: response.status,
response: await response.json(),
},
'[ads] Web API returned error',
)
continue
}
Comment on lines +309 to +319
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent no-fill fallback makes debugging harder

When the primary provider returns a 200 with no ad content (neither data.ad nor a valid data.ads array), the loop falls through to the fallback silently — no log line is emitted. Adding a brief logger.debug here would make it easy to spot "primary returned no fill, trying fallback" in logs without changing behaviour:

Suggested change
if (!response.ok) {
logger.warn(
{
provider: providerToTry,
status: response.status,
response: await response.json(),
},
'[ads] Web API returned error',
)
continue
}
if (data.ad) {
return { variant: 'banner', ad: data.ad as AdResponse }
}
// No fill from this provider; try the next one if available
logger.debug({ provider: providerToTry }, '[ads] No fill, trying next provider')
Prompt To Fix With AI 
This is a comment left during a code review.
Path: cli/src/hooks/use-gravity-ad.ts
Line: 309-319

Comment:
**Silent no-fill fallback makes debugging harder**

When the primary provider returns a 200 with no ad content (neither `data.ad` nor a valid `data.ads` array), the loop falls through to the fallback silently — no log line is emitted. Adding a brief `logger.debug` here would make it easy to spot "primary returned no fill, trying fallback" in logs without changing behaviour:

```suggestion
        if (data.ad) {
          return { variant: 'banner', ad: data.ad as AdResponse }
        }

        // No fill from this provider; try the next one if available
        logger.debug({ provider: providerToTry }, '[ads] No fill, trying next provider')
```

How can I resolve this? If you propose a fix, please make it concise.


if (variant === 'choice' && Array.isArray(data.ads) && data.ads.length > 0) {
return { variant: 'choice', ads: data.ads as AdResponse[] }
}
const data = await response.json()
const variant = data.variant ?? 'banner'

if (data.ad) {
return { variant: 'banner', ad: data.ad as AdResponse }
}
if (
variant === 'choice' &&
Array.isArray(data.ads) &&
data.ads.length > 0
) {
return { variant: 'choice', ads: data.ads as AdResponse[] }
}

return null
} catch (err) {
logger.error({ err }, '[ads] Failed to fetch ad')
return null
if (data.ad) {
return { variant: 'banner', ad: data.ad as AdResponse }
}
} catch (err) {
logger.error({ err, provider: providerToTry }, '[ads] Failed to fetch ad')
}
}

return null
}

// Update tick function (uses ref to avoid useCallback dependency issues)
Expand Down Expand Up @@ -413,7 +430,7 @@ export const useGravityAd = (options?: {
clearInterval(id)
ctrlRef.current.intervalId = null
}
}, [shouldStart, shouldHideAds])
}, [shouldStart, shouldHideAds, provider, fallbackProvider])

// Don't return ad when ads should be hidden
const visible = shouldStart && !shouldHideAds
Expand Down
Loading