PubstackAdapter: add default value for tll & netRevenue#14637
PubstackAdapter: add default value for tll & netRevenue#14637lksharma merged 13 commits intoprebid:masterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8163b8da2c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const mapResponseBidsByImpId = (responseBody) => new Map<string, OpenRtbBid>( | ||
| (responseBody?.seatbid ?? []) | ||
| .flatMap(seatBid => seatBid?.bid ?? []) | ||
| .filter(bid => bid?.impid) | ||
| .map(bid => [bid.impid, bid]), |
There was a problem hiding this comment.
This mapping collapses all raw bids that share the same impid into a single entry, so when a response contains multiple bids for one impression (e.g., multiple seat bids on the same imp), enrichBidResponse() will apply the last raw bid’s ttl/netRevenue to every converted bid with that requestId. In those cases, earlier bids get incorrect metadata, which can change bid expiry behavior and net/gross revenue interpretation.
Useful? React with 👍 / 👎.
Coverage Report for CI Build 24089107179Coverage increased (+0.001%) to 96.343%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
| const enrichBidResponse = (adapterResponse, responseBody) => { | ||
| const rawBidsByImpId = mapResponseBidsByImpId(responseBody); | ||
|
|
||
| return { | ||
| ...adapterResponse, | ||
| bids: (adapterResponse?.bids ?? []).map(bid => { | ||
| const rawBid = rawBidsByImpId.get(bid.requestId); | ||
| return { | ||
| ...bid, | ||
| ttl: bid.ttl ?? rawBid?.ttl ?? rawBid?.exp ?? DEFAULT_TTL, | ||
| netRevenue: bid.netRevenue ?? rawBid?.netRevenue ?? DEFAULT_NET_REVENUE, | ||
| }; | ||
| }), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
You could do this using ortbConverter's bidResponse, something like
bidResponse(buildBidResponse, bid, context) {
return {
...buildBidResponse(bid, context),
ttl: bid.ttl ?? bid.exp ?? DEFAULT_TTL,
netRevenue: bid?.netRevenue ?? DEFAULT_NET_REVENUE,
}
}
which would make it considerably simpler & shorter, and also avoid the bot's concern. I'll note that neither ttl nor netRevenue are part of the ORTB spec and ortbConverter already uses exp when it finds it.
There was a problem hiding this comment.
Thanks, it is indeed much shorter
5 participants
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org/?
Other
Description of change
This PR fixes Pubstack bid response enrichment for OpenRTB responses that provide
ttlandnetRevenuefields outside of the default ORTB converter mapping.The Pubstack adapter already relies on the ORTB converter for standard bid response fields, but some exchanges return:
seatbid[].bid[].ttlinstead ofseatbid[].bid[].expseatbid[].bid[].netRevenueinstead of values supplied through converter contextTo support these responses, the adapter enriches converted bids after
fromORTB()by matching raw bids byimpidand backfilling:ttlfromrawBid.ttl, thenrawBid.exp, then the adapter defaultnetRevenuefromrawBid.netRevenue, then the adapter defaultThis prevents valid Pubstack bids from being rejected by Prebid validation when these required fields are missing after ORTB conversion.
The change is scoped to the Pubstack bid adapter and includes test coverage for response mapping.
Other information