Always emit hb_adid so server-side ad template creatives render - #996
Merged
Conversation
…e absent Bidders that return neither a Prebid Cache UUID nor an `adid` produced no `hb_adid` in `window.tsjs.bids` at all. `adInit` only sets targeting keys that exist on the bid, so GAM never received an `hb_adid` key, the Universal Creative's `%%PATTERN:hb_adid%%` expanded to empty, and the render bridge rejected the resulting `Prebid Request` message for want of an ad ID. The line item won and served its wrapper, but the creative never rendered. Add `Bid::bid_id`, populated from the OpenRTB bid object's own `id`, and use it as the last-resort `hb_adid` source. Per spec `id` is mandatory, so this closes the gap for every bidder. It is unique per bid instance rather than a creative identifier, which is exactly what `hb_adid` needs here: a stable value GAM echoes back verbatim so the bridge can find this winning bid. `cache_id` and `ad_id` keep priority in that order — locked in by test, since the Universal Creative treats `hb_adid` as the Prebid Cache lookup key whenever `hb_cache_host`/`hb_cache_path` are present. `bid_id` is carried as its own field rather than folded into `ad_id`, which is exposed raw in the debug bid and would mislead consumers treating it as a creative identifier. Verified: cargo fmt, all six clippy targets, test-fastly / test-axum / test-cloudflare / test-spin, the parity suite, JS vitest, and JS + docs prettier checks all pass. The new bid_map test was confirmed to fail with the fallback removed.
prk-Jr
requested review from
ChristianPavilonis and
aram356
and removed request for
aram356
August 4, 2026 16:51
ChristianPavilonis
approved these changes
Aug 5, 2026
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Review Summary
Reviewed PR #996 against main. The direct Prebid fallback is correctly wired, but the inline findings cover an incomplete mediated path and a defensive fallback edge case.
aram356
requested changes
Aug 6, 2026
aram356
left a comment
aram356
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Well-executed, tightly scoped fix: the cache_id → ad_id → bid_id precedence is locked in by tests from both directions, the new fallback test was verified to fail without the fix, and the render loop stays symmetric end-to-end (build_bid_map → GPT targeting → %%PATTERN:hb_adid%% → bridge equality check). One blocking gap on the mediated path, plus a few non-blocking observations.
Blocking
🔧 wrench
- Mediation passthrough drops
bid_id— see inline comment (crates/trusted-server-core/src/integrations/adserver_mock.rs:321).
Non-blocking
🤔 thinking
- GAM 40-char targeting-value limit — see inline comment (
crates/trusted-server-core/src/publisher.rs:3293). hb_cache_host/hb_cache_pathare emitted independently ofcache_id: if PBS ever returnsext.prebid.cache.bids.urlwithoutcacheId, the map now carrieshb_adid = bid_idalongside cache coordinates, and (absent inlineadm) the Universal Creative would fetchhttps://<host><path>?uuid=<bid_id>— a guaranteed miss. Pre-existing with thead_idfallback, butbid_idextends it to every bidder. A cheap guard is to gate thehb_cache_host/hb_cache_pathinserts oncache_id.is_some()(crates/trusted-server-core/src/publisher.rs:3300-3315).
🌱 seedling
/auctionresponses could now echo the real OpenRTB bid id:crates/trusted-server-core/src/auction/formats.rs:281synthesizesid: Some(format!("{}-{}", bid.bidder, slot_id)). Now thatBidcarries the upstream bid's ownid, echoingbid.bid_id(with the synthetic as fallback) would preserve win-notification correlation and debugging traceability for/auctionconsumers. Follow-up scope, not this PR.
⛏ nitpick
- Empty-string
idflows into the map — see inline comment (crates/trusted-server-core/src/integrations/prebid.rs:2228). - Stale comment in the JS bridge:
crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1162-1164explainshb_adidnon-uniqueness solely via the creative-id fallback; there is now a third tier (bid_id, unique per bid instance). The slot-scoped lookup rationale still holds, but the comment is incomplete. File isn't in this diff, hence body-level.
CI Status
All checks pass on GitHub: fmt, clippy (all six target-matched variants), Rust tests (fastly/axum/cloudflare/spin/CLI), cross-adapter parity, vitest, browser and Fastly EC integration tests, docs/TS format, CodeQL.
Carry bid_id through adserver_mock mediation. The reconstruction restored
nurl, burl, ad_id and the cache fields from the original SSP bid but
hard-coded bid_id to None, so a mediated bid whose only hb_adid source is
the OpenRTB bid id lost it and never rendered — the exact failure this
branch fixes, re-opened on the mediated path. Mediated APS bids were
worse off still: they carry no ad_id or cache_id for the restore to
recover, so they reached the page with no hb_adid at all. The mediation
response is itself OpenRTB, so prefer the mediated bid's own id and fall
back to the original SSP bid's.
Reject blank identity strings before applying the cache_id -> ad_id ->
bid_id precedence. Option::or treats Some("") as present, so a blank
cacheId or adid outranked a valid bid id and emitted an empty hb_adid —
falsey on the page, so GPT skips the targeting key and the render bridge
has nothing to match. Prebid's parse_bid likewise treats an empty
OpenRTB id as absent.
Emit hb_cache_host and hb_cache_path only alongside a real Prebid Cache
UUID. PBS reports the cache url and cacheId independently, so a bid with
coordinates but no UUID pointed the Universal Creative at
?uuid=<non-cache-id>, a guaranteed miss, instead of letting it fall
through to the inline adm.
Warn when the chosen hb_adid exceeds GAM's 40-character targeting value
limit. GAM drops an over-long value, so the creative echoes nothing and
the bridge's equality check never matches. Log rather than truncate: a
truncated id is no longer unique per bid, which is what lets one slot's
render claim another slot's creative.
prk-Jr
added a commit
that referenced
this pull request
Aug 6, 2026
Brings the PR #996 review fixes onto rc/july. The original fallback commit was already merged; this adds the follow-ups plus two conflict resolutions where rc/july had since moved. hb_adid precedence keeps rc/july's renderer tier and gains blank rejection on every tier, so a bidder emitting an empty cacheId or adid no longer outranks a usable identifier: non_empty(cache_id) -> non_empty(renderer_bid_id) -> non_empty(ad_id) -> non_empty(bid_id) Cache coordinates keep rc/july's placement inside the processed_adm match and gain the cache_id requirement as a second, independent condition: absent a UUID the coordinates would send the Universal Creative to ?uuid=<non-cache-id>. Mediation bid_id precedence is inverted relative to the main-targeted branch, deliberately. On main, APS bids carry no bid_id, so preferring the mediation response's own id was the only way to give them an hb_adid. On rc/july, aps.rs populates bid_id and a typed renderer envelope is minted against it, and build_bid_map derives hb_adid from that pairing — so substituting the mediator's id would key targeting to an id the renderer does not know. The original SSP bid's id therefore wins here, with the mediated id as the fallback for an upstream bid that carried none.
aram356
approved these changes
Aug 7, 2026
The gate used bid.cache_id.is_some() while hb_adid used non_empty(). PBS maps cacheId straight through with no emptiness filter and parses the cache url independently, so a bid carrying cacheId: "" alongside a parseable url passed the gate but lost the hb_adid precedence to adid or the bid id. The map then shipped cache coordinates against a non-cache hb_adid, pointing the Universal Creative at ?uuid=<adid> — the guaranteed cache miss the gate exists to prevent — instead of letting it fall through to the inline adm.
prk-Jr
added a commit
that referenced
this pull request
Aug 7, 2026
Conflict in the Prebid refresh handler: main (#965) documented that the delegated refresh preserves the publisher's original bare form, while this branch replaced that call with the diagnostics recording plus the scoped dispatch context. Both hold — `dispatchPrebidRefresh` passes `slots` and `opts` through unchanged — so the resolution keeps the diagnostics calls and main's comment, extended to say the wrapper only scopes the shared context. Also add `bid_id` to the auction-ID test provider's `Bid` literal, a field main added in #996 after this branch introduced the provider.
prk-Jr
added a commit
that referenced
this pull request
Aug 7, 2026
Brings in the PR #997 review fixes: the read-only diagnostics facade split from the internal recorder channel, lazily expiring request-intent evidence with one shared delivery-boundary timer, source-agnostic IDs no longer reported as reservations, the empty-render attempt eviction, and the scoped type-check gate for the export contract. The branch also carries a merge of main, whose #965 and #996 arrive here as squashes of work rc/july already implements more fully. Where the two sides describe the same feature, rc/july's implementation is kept: - APS, adserver_mock, auction/types.rs, auction/formats.rs — rc/july's OpenRTB provider, renderer-aware bid_id precedence, and typed renderer envelope supersede main's versions, which drop fields rc/july needs. - prebid.rs — rc/july canonicalizes the excluded-suffix list at both the startup and build paths already, so main's `load_config` helper adds nothing. Main's test is taken instead of rc/july's: it builds from raw settings rather than reusing the config `validate_config_for_startup` already canonicalized, so it actually exercises the build path. - prebid/index.ts — a bare refresh that filtered slots must deliver the resolved target list, not stay bare, so rc/july's `deliveredSlots` behavior and its test expectation both stand. Three fixes are ported into rc/july's shapes rather than resolved away: - The EC-derived auction ID reached page JavaScript here too, through different plumbing: both collect paths inlined `request.id.as_str()` into `write_bids_to_state`, and page-bids passed it to `build_bid_map_with_auction_id`. All three now mint a per-auction token via `diagnostics_auction_id()`, gated on the diagnostics integration being enabled. - A blank Prebid Cache UUID no longer ships cache coordinates. It loses the hb_adid precedence to `adid` or the bid id, so the Universal Creative would fetch `?uuid=<non-cache-id>` and miss instead of using the inline adm. The gate moves from `is_some()` to `non_empty()`, and main's regression test comes along with rc/july's `Bid` fields added. - The browser-side excluded-suffix list is validated before use. The server only de-duplicates it, so an empty suffix matched every ad unit path and pulled every slot out of the refresh auction, and a non-array value threw inside the publisher's own `refresh()`. Also collapses a duplicated `hb_auction_id` write in `build_bid_map` down to one guarded insert, and points the Prebid refresh recorder at `gptDiagnosticsRecorder` to match the new channel.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
adidproduced nohb_adidinwindow.tsjs.bids.adInitonly sets targeting keys that exist on the bid, so GAM never received anhb_adidkey, the Universal Creative's%%PATTERN:hb_adid%%expanded to empty, and the render bridge rejected the resultingPrebid Requestfor want of an ad ID. The line item won and served its wrapper, but the creative never rendered — a silent, per-bidder loss of every SSAT impression.Bid::bid_id, populated from the OpenRTB bid object's ownid, and uses it as the last-resorthb_adidsource.idis mandatory per the OpenRTB spec, so this closes the gap for every bidder.cache_idandad_idkeep priority, in that order. This is not cosmetic: whenhb_cache_host/hb_cache_pathare present the Universal Creative useshb_adidas the Prebid Cache lookup key (https://<host><path>?uuid=<hb_adid>), so an arbitrary value there would break cache retrieval. Both priorities are locked in by test.Why a separate field rather than folding into
ad_idad_idis the creative/ad identifier and is exposed raw in the debug bid. The OpenRTBidis unique per bid instance, not per creative — surfacing it asad_idwould mislead any consumer treating that field as a creative identifier. The existing comment inparse_bidsays as much, so the bid ID is carried as its own field instead.Changes
crates/trusted-server-core/src/auction/types.rsBid::bid_id: Option<String>with#[serde(default, skip_serializing_if = "Option::is_none")], so the serialized wire shape is unchanged when absentcrates/trusted-server-core/src/integrations/prebid.rsbid_idfrom the OpenRTB bididinparse_bid; two tests covering it alongside and withoutadid/cachecrates/trusted-server-core/src/publisher.rsbuild_bid_maphb_adidchain becomescache_id → ad_id → bid_id; test for the new fallback, renamed omit-case test, andbid_idadded to the cache/ad-id tests to prove precedencecrates/trusted-server-core/src/auction/orchestrator.rsbid_id: NoneatBidliterals (tests)crates/trusted-server-core/src/auction/formats.rsbid_id: NoneatBidliteralcrates/trusted-server-core/src/auction/telemetry.rsbid_id: NoneatBidliteral (test)crates/trusted-server-core/src/integrations/adserver_mock.rsbid_id: NoneatBidliteralscrates/trusted-server-core/src/integrations/aps.rsbid_id: NoneatBidliteralCHANGELOG.mdTest plan
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servecargo test-cloudflare,cargo test-spin,cargo clippy-cloudflare,cargo clippy-cloudflare-wasm,cargo clippy-spin-native,cargo clippy-spin-wasm, and the parity suite (cargo test --manifest-path crates/trusted-server-integration-tests/Cargo.toml --test parity)The new
bid_map_falls_back_to_bid_id_when_cache_id_and_ad_id_absenttest was confirmed to fail with the fallback removed (left: None), so it genuinely gates the fix rather than passing vacuously.Not covered by automated tests: the browser-side half of the loop — that GAM echoes the new
hb_adidback and the bridge serves the creative. That path is the subject of #926.Checklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)