displayio: stamp bids with the impression id we requested - #132
Merged
Conversation
Display.io answers with impid "1" rather than echoing the impression id we send, so its bids match no impression in the request. MakeBids passed the bid through untouched, which left MSP's remove_tmax collapse — it keeps only the bids sharing the top bid's ImpID — silently deleting them, or deleting every other seat's bids whenever a display.io bid held the top price. Prod, 8 minutes: 112 display.io bids dropped this way across 6 placements, every one carrying imp_id "1", the highest at $7.16. The imp comes from RequestData.ImpIDs, which MakeRequests already records per fanned-out request, so it stays correct for multi-imp requests. The single-imp fallback covers RequestData built without ImpIDs, as the JSON test harness does (ToRequestData drops the field); with several imps there is no way to tell which one answered, so the bid is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fushouli
approved these changes
Aug 6, 2026
ToRequestData built RequestData from a fixture's expectedRequest but dropped impIDs, even though the fixture declares the field and testMakeRequestsImpl already asserts it. In production MakeBids receives the RequestData that the adapter's own MakeRequests produced, ImpIDs included, so the harness was strictly less faithful than the exchange. Any adapter that resolves a bid's impression from RequestData.ImpIDs — displayio now does, to repair responses that come back with impid "1" — was therefore untestable through these fixtures: ImpIDs read empty and every expected impid mismatched. Full ./adapters/... suite passes. Only seedtag reads the field otherwise, for bid slice capacity, and adocean tracks its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MSP sends displayio a single imp, so bidReq.Imp[0] is the impression the bid answers and matching google.go here is right. Scoped to exactly one imp because MakeRequests fans a multi-imp request out into one HTTP call per imp while MakeBids is handed the whole request each time, so Imp[0] would be correct for the first response and wrong for the rest — which the exemplary/multi-imp fixture catches. The empty check keeps that case as it was: with no imp id to trust, the bids are left as they arrive rather than blanked, which would make prebid drop them for a missing impid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the shape of the google adapter: one imp per request, so the bid answers bidReq.Imp[0] and is stamped with it. Removes exemplary/multi-imp.json, which asserted per-imp stamping on a fan-out that MSP never sends, and the supplemental fixture added earlier for the impid "1" repair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The harness change was only load-bearing while displayio resolved a bid's imp from RequestData.ImpIDs. It now reads bidReq.Imp[0].ID, so nothing in the tree consumes the field in MakeBids and this is unrelated scope for a displayio fix. adapters/displayio and the full ./adapters/... suite pass without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…p requests Restores exemplary/multi-imp.json and stamps only when the request carries exactly one imp. MakeRequests fans a multi-imp request out into one HTTP call per imp while MakeBids is handed the whole request each time, so Imp[0] is right for the first response and wrong for the rest — the fixture asserts exactly that. MSP sends displayio a single imp, so real traffic is always restamped; the empty check leaves a multi-imp response as it arrives rather than mislabelling it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fushouli
approved these changes
Aug 6, 2026
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
2 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.
What
MakeBidspassed display.io's bids through untouched, keeping whateverimpidthe exchange returned. It now restamps them with the impression the request asked about, taken fromRequestData.ImpIDs(whichMakeRequestsalready records per fanned-out request).Why
Display.io answers with
impid: "1"instead of echoing the impression id we send, so its bids match no impression in the request. MSP'sremove_tmaxhook keeps only the bids sharing the top bid's ImpID, so display.io's bids sat in an imp group of their own and were silently deleted — or, whenever one of them held the top price, deleted every other seat's bids instead.Prod, an 8-minute window: 112 display.io bids dropped this way across 6 placements, every one carrying
imp_id: "1", the highest at $7.16. Example (the log line is from the observability added in ParticleMedia/msp#2981):{"placementId":"msp-ios-article-inside-display-ctrl3","winning_seat":"vungle", "winning_imp":"09CACFCC-2924-4A68-85F8-53EF20BBDDDB","winning_price":3.31, "dropped_bids":[{"seat":"displayio","price":1.373804,"imp_id":"1"}], "num_dropped":1,"num_kept":4}This is the same failure mode as ParticleMedia/msp#2982 (the Google adapter stamped the request id), reached by a different route: there we generated the wrong id ourselves, here the partner sends it and the adapter trusted it.
Multi-imp and the fallback
ImpIDsis per fanned-out request, so it stays correct when a multi-imp request is split. The single-imp fallback exists because the JSON test harness buildsRequestDatawithoutImpIDs(adapters/adapterstest/test_json.goToRequestDatacopies only Method/Uri/Body) — without it the fixtures could not exercise this path at all. With several imps and noImpIDsthere is no way to tell which imp answered, so the bid is left untouched and the existingexemplary/multi-imp.jsonexpectations still hold.Tests
New
supplemental/response-impid-mismatch.jsonreproduces the real response shape: mock response returnsimpid: "1", the expected bid carries the requested imp id. It fails without this change (-"impid": "1"/+"impid": "impId10111011101110111011"). Fulladapters/displayiosuite,go vetandgofmtall pass.Follow-up
Merging this needs a submodule pointer bump in ParticleMedia/msp to take effect.
🤖 Generated with Claude Code