Skip to content

Unicorn Bid Provider: add slot position & viewability module - #15379

Merged
patmmccann merged 5 commits into
prebid:masterfrom
bulbit:unicorn-rtd-provider
Aug 8, 2026
Merged

Unicorn Bid Provider: add slot position & viewability module#15379
patmmccann merged 5 commits into
prebid:masterfrom
bulbit:unicorn-rtd-provider

Conversation

@harufujimoto

@harufujimoto harufujimoto commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Type of change

  • Updated bidder adapter (Unicorn Bid Adapter measures and sends slot position/geometry/viewability)

Description of change

For each bid request, the Unicorn Bid Adapter measures the ad slot's on-screen
position/geometry and viewability, and sends it in the OpenRTB payload it
builds. This is scoped to the adapter's own request only — nothing is written
to shared First Party Data, so no other bidder or PBS is affected.

  • imp.banner.pos — OpenRTB AdPosition (1 = above the fold, 3 = below the
    fold). A publisher-declared ortb2Imp.banner.pos, if present, is used
    instead of the measured value.
  • imp.ext.unicorn{ ver, ratio, fixed, sticky, w, h, x, y }, where
    ratio is the visible-area ratio (0–1), fixed/sticky report a
    fixed/sticky ancestor, and x/y/w/h are the slot's document-relative
    position and rendered size in CSS pixels.

Slot element resolution order: ortb2Imp.ext.data.divId → GPT
getSlotElementId() → the ad unit code.

Module maintainer

Test parameters

{
  bidder: 'unicorn',
  params: {
    accountId: 12345
  }
}

Other information

  • Docs: modules/unicornBidAdapter.md.
  • Tests: test/spec/modules/unicornBidAdapter_spec.js covers the position/
    geometry/viewability measurement.
  • A follow-up PR will add imp.ext.gpid support to the Unicorn Bid Adapter;
    it is kept separate to stay scoped to one change per PR.

…als) (#1)

* New RTD Module: Unicorn Viewability (slot position + viewability signals)

- Add unicornViewabilityRtdProvider: measure each ad slot's on-screen
  position and visibility ratio on the client, inject into
  ortb2Imp.ext.data.unicorn and the standard ortb2Imp.banner.pos.
- unicornBidAdapter: forward the measured signal to the wire as
  imp.ext.unicorn (flat vendor key), and pass through imp.banner.pos.
- Add integration example page (integrationExamples/gpt).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Unicorn Viewability RTD Module: add docs and unit tests

- modules/unicornViewabilityRtdProvider.md: module overview, integration,
  configuration, slot element resolution, injected-field table.
- test/spec/modules/unicornViewabilityRtdProvider_spec.js: cover init,
  getBidRequestData injection (ver 1 / banner.pos), missing-slot and
  empty-adUnits cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Unicorn RTD Module: rename to unicornRtdProvider, address review feedback

- Rename unicornViewabilityRtdProvider -> unicornRtdProvider (module, md,
  spec); submodule name 'unicornViewability' -> 'unicorn'; export
  unicornViewabilitySubmodule -> unicornSubmodule.
- unicornBidAdapter: rename local var unicornViewability -> unicornSignal.
- Drop the integration example page from the PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Unicorn RTD Module: name the signal namespace imp.ext.adslot

Rename the injected/wire key from the vendor name `unicorn` to `adslot`,
reflecting that it carries the ad slot's position/geometry (x/y/w/h/fixed)
plus visibility ratio. Internal ortb2Imp.ext.data.adslot, wire imp.ext.adslot.
Also rename the adapter local var unicornSignal -> adslotSignal, and update
the module doc and unit tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Unicorn RTD Module: map fixed/sticky slots to banner.pos=2 (AdCOM Locked)

Per AdCOM 1.0 Placement Positions (used by prebid/openrtb v20, which alicorn
follows), position 2 = Locked (fixed position). Emit banner.pos=2 for
fixed/sticky slots instead of only 1/3; keep the adslot.fixed flag as the raw
signal. Update the module doc and add a unit test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings July 23, 2026 06:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Unicorn Real-Time Data (RTD) provider that measures per-slot viewability/position client-side and injects signals into adUnit.ortb2Imp for downstream bidders to consume (modules/unicornRtdProvider.js:1-160). Updates the Unicorn bid adapter to forward those injected signals into the OpenRTB imp object (modules/unicornBidAdapter.js:49-71), and includes initial unit test coverage plus module documentation (test/spec/modules/unicornRtdProvider_spec.js:1-102, modules/unicornRtdProvider.md:1-76).

Changes:

  • Add unicornRtdProvider RTD submodule to compute and inject ortb2Imp.banner.pos and ortb2Imp.ext.data.adslot (modules/unicornRtdProvider.js:63-160).
  • Forward injected banner.pos and ext.data.adslot from bid requests into OpenRTB imp.banner.pos and imp.ext.adslot (modules/unicornBidAdapter.js:49-71).
  • Add docs and unit tests covering initialization, slot resolution, and injection behavior (modules/unicornRtdProvider.md:1-76, test/spec/modules/unicornRtdProvider_spec.js:1-102).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
modules/unicornRtdProvider.js New RTD submodule: resolves slot element, measures geometry/viewability, injects ortb2Imp signals pre-auction.
modules/unicornBidAdapter.js Reads RTD-injected ortb2Imp values and forwards them onto the OpenRTB imp payload.
test/spec/modules/unicornRtdProvider_spec.js Adds unit tests for init, slot resolution, and signal injection behavior.
modules/unicornRtdProvider.md Adds module documentation and integration/configuration details.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/unicornRtdProvider.js Outdated
Comment on lines +28 to +29
// adUnitCode(=div id) -> { ratio, fixed, slotPosition }
const measurements = {};
Comment thread modules/unicornRtdProvider.js Outdated
Comment on lines +41 to +43
observer = new IntersectionObserver(handleIntersection, {
threshold: [0, 0.25, 0.5, 0.75, 1]
});
Comment thread modules/unicornRtdProvider.md Outdated
Comment on lines +3 to +4
Module Name: Unicorn Rtd Provider
Module Type: Rtd Provider

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc92f48ba1

ℹ️ 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".

Comment thread modules/unicornRtdProvider.js Outdated
Comment thread modules/unicornRtdProvider.js Outdated
// start observing for ongoing updates (idempotent enough for a PoC)
try { observer && observer.observe(el); } catch (e) { /* noop */ }

const rect = getBoundingClientRect(el);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bypass stale cached slot geometry

This provider is invoked from RTD's startAuction.before(..., 20) hook, but the shared getBoundingClientRect helper clears its per-element cache in its own lower-priority startAuction.before hook. When any other bundled module cached the same slot during the previous auction, refreshed auctions read that stale rectangle here before the cache is cleared, so the emitted x/y/w/h, ratio, and banner.pos can describe the prior layout instead of the current one; use the raw DOM API or clear the helper cache before measuring.

Useful? React with 👍 / 👎.

Copilot AI review requested due to automatic review settings July 23, 2026 17:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@patmmccann

Copy link
Copy Markdown
Collaborator

why not drop this rtd module and have the unicorn bid adapter import the viewability measurements coming from the existing library?

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!

  • modules/unicornRtdProvider.js

Tread carefully! This PR adds 1 linter error (possibly disabled through directives):

  • modules/unicornRtdProvider.js (+1 error)

@github-actions

Copy link
Copy Markdown

This PR includes an adapter whose code does not match its file name. Bid adapter modules should be named <bidderCode>BidAdapter, userId <userIdCode>IdSystem, RTD <rtdCode>RtdProvider, and analytics <analyticsCode>AnalyticsAdapter.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:

  • Changes to test/spec/modules/unicornBidAdapter_spec.js may need:
    • es.array.push

The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers.

@barecheck

barecheck Bot commented Jul 28, 2026

Copy link
Copy Markdown

Barecheck - Code coverage report

Total: 91.14%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

- Source the visibility ratio from Prebid's shared `percentInView` helper and
  drop the module's own IntersectionObserver. This removes the DOM-id-keyed map
  and observer lifecycle flagged in review (prototype pollution / observer
  leak) by removal rather than patching.
- Read getBoundingClientRect directly for geometry so the shared cached helper
  can't return a prior-auction rectangle here.
- Register unicornRtdProvider under rtdModule in modules/.submodules.json so
  `--modules=unicornRtdProvider` pulls in rtdModule.
- Docs: consistent "RTD" capitalization; note the visibility-ratio source.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 30, 2026 07:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

modules/unicornRtdProvider.js:126

  • getBidRequestData currently ignores the RTD core’s timeout argument and may wait for DOMContentLoaded + requestAnimationFrame even after the auctionDelay has expired. When that happens, it can inject ortb2Imp signals after the auction has already continued (or even into a later auction), which is inconsistent with the “within auctionDelay budget” comment and makes behavior timing-dependent.

Consider accepting the timeout parameter (5th arg) and adding a cancellation guard that (a) calls callback() on timeout and (b) prevents late measurement/injection once timed out.

function getBidRequestData(reqBidsConfigObj, callback) {
  const measureAndDone = () => {
    (reqBidsConfigObj.adUnits || []).forEach(adUnit => {
      const divId = resolveDivId(adUnit);
      const m = measureNow(divId);

modules/unicornBidAdapter.js:70

  • This change introduces new wire behavior (passthrough of ortb2Imp.banner.posimp.banner.pos and ortb2Imp.ext.data.adslotimp.ext.adslot), but the existing adapter unit tests don’t exercise either branch (no adslot/banner.pos assertions, and no fixtures include these fields). Adding a focused test case would help keep this behavior stable and aligns with the repo’s coverage expectations for modified code.

For example, extend the unicorn adapter spec to include a bidRequest fixture with ortb2Imp.banner.pos and ortb2Imp.ext.data.adslot, and assert the built OpenRTB payload includes imp[n].banner.pos and imp[n].ext.adslot.

    const adslotSignal = deepAccess(br, 'ortb2Imp.ext.data.adslot');
    const pos = deepAccess(br, 'ortb2Imp.banner.pos');
    const banner = {
      format: makeFormat(br.sizes),
      w: br.sizes[0][0],
      h: br.sizes[0][1]
    };
    if (pos != null) {
      banner.pos = pos;
    }
    const impObj = {
      id: br.bidId,
      banner,
      tagid: deepAccess(br, 'params.placementId') || br.adUnitCode,
      secure: 1,
      bidfloor: parseFloat(0)
    };
    if (adslotSignal) {
      // wire contract: imp.ext.adslot (flat key, received like skadn) — see spec
      impObj.ext = { adslot: adslotSignal };
    }

@harufujimoto

Copy link
Copy Markdown
Contributor Author

Hi @patmmccann; Thanks for the review, and a quick follow-up on the updates I pushed.

Per your suggestion, the module no longer runs its own IntersectionObserver — the visibility ratio now comes from the shared percentInView helper. It keeps the parts that helper doesn't cover (the slot's document-relative position and size, the fixed/sticky flag, and the OpenRTB banner.pos), which are the module's main purpose. I kept it as an RTD provider for the pre-auction timing, but I'm happy to restructure it inline in the adapter if you'd prefer.

Could you let me know if anything else is needed to move this forward? Happy to make further changes. Thanks!

Comment thread modules/unicornRtdProvider.js Outdated
if (m) {
deepSetValue(adUnit, `ortb2Imp.ext.data.${ORTB2_NAMESPACE}`, m.signal);
// standard OpenRTB ad position lives in banner.pos, not in ext.adslot
deepSetValue(adUnit, 'ortb2Imp.banner.pos', m.pos);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This ignores a publisher-provided pos if there is one. Should it?

Comment thread modules/unicornRtdProvider.js Outdated
if (!el) return null;

// Read current geometry directly. The shared getBoundingClientRect helper
// caches per-auction and could return a prior-auction rectangle here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This turns out to be a bigger problem than I anticipated - opened #15446 to address it separately.

If you want this measurement to be done here (as opposed to the bid adapter), my suggestion is to still use the getBoundingClientRect helper, which should automatically pick up the fix for #15446. As of right now that'd behave worse than this version, but this version is still susceptible to the "problem 2" described in that issue.

However, measuring here means that - depending on how RTD is configured - the page can have enough time to change before the bid adapter runs. If for example RTD is set up with auctionDelay: 500 and the user is scrolling during that half-second viewability can be very different when measured from here vs from a bid adapter. The other signals are less likely to change but not immune to this problem.

@dgirardi dgirardi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Most of what follows is about where these signals are written rather than how they are measured. Because the module writes to adUnit.ortb2Imp, everything it produces is merged into imp for every bidder on the page (libraries/ortbConverter/processors/default.js:41-43, priority 99), not just for Unicorn. Two findings are cross-bidder breakage that follows directly from that — the ext.data.adslot key collision (line 109) and the ungated banner.pos write (line 111) — and they look like the ones to resolve before this lands.

On the module's structure: resolveDivId does mirror adagioRtdProvider, as the comment at line 40 says, but measureNow is a second, cruder implementation of that module's getSlotPosition (adagioRtdProvider.js:497-558) — it drops safeframe support, cross-frame element resolution, clientTop/clientLeft correction and the display: none warning, and it has the window mismatch noted on line 61. Rather than reimplementing it, consider extracting adagio's version into libraries/ and importing from both; PR_REVIEW.md asks for shared code over duplication, and it would resolve several of the inline findings at once.

One item on the PR description: it states "adapter tests cover the pos / adslot passthrough". They don't — test/spec/modules/unicornBidAdapter_spec.js is not in the diff and contains no reference to adslot, banner.pos or ortb2Imp. The barecheck comment independently flags unicornBidAdapter.js:58 and :69 — the two new branches — as uncovered. Adapter tests for pos present/absent and adslot present/absent would meet the 80%-on-changed-code bar.

Finally, connected to the earlier question about dropping the RTD module, and distinct from the measurement-timing thread on line 58: the reason this module causes cross-bidder side effects at all is that RTD's only channel here is shared FPD, while the data has exactly one consumer. If the signals moved into the Unicorn adapter, the first three findings disappear rather than needing fixes.

Separately, on labels: this currently has only the core label. It needs a release label (feature) and a SemVer label (minor).

The findings in this review and in its inline comments were generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
const divId = resolveDivId(adUnit);
const m = measureNow(divId);
if (m) {
deepSetValue(adUnit, `ortb2Imp.ext.data.${ORTB2_NAMESPACE}`, m.signal);

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ext.data.adslot is already taken, and it holds a string — the GAM ad slot path. Writing an object there breaks other bidders in the same bundle:

  • modules/beopBidAdapter.js:274 reads it as one of its name fallbacks (ortb2Imp.ext.gpid || ortb2Imp.ext.data.adslot || ortb2Imp.ext.data.adserver.adslot || …), so with this module installed and no gpid configured, BeOP's slot name becomes {ver: 1, ratio: …, fixed: …, w: …, h: …, x: …, y: …} and its slot identification breaks.
  • modules/rubiconBidAdapter.js:896-898 forwards every non-adserver ext.data.* key as a Rubicon tg_i.* key-value, so it emits a junk adslot target.

And because ortb2Imp is merged into imp for all ORTB bidders and PBS, the blob goes out as imp.ext.data.adslot everywhere, not only to Unicorn.

Suggest namespacing it — ortb2Imp.ext.data.unicorn — and updating the adapter and the .md table to match. ORTB2_NAMESPACE on line 24 is the single place to change.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
if (m) {
deepSetValue(adUnit, `ortb2Imp.ext.data.${ORTB2_NAMESPACE}`, m.signal);
// standard OpenRTB ad position lives in banner.pos, not in ext.adslot
deepSetValue(adUnit, 'ortb2Imp.banner.pos', m.pos);

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Separate from the earlier comment on this same line about overwriting a publisher-provided pos: this writes banner.pos unconditionally, with no mediaType check. For a video-only or native-only ad unit the ortb2Imp merge creates imp.banner = {pos: N}, and fillBannerImp then skips the whole block because bidRequest.mediaTypes.banner is absent (libraries/ortbConverter/processors/banner.js:18-19), so that bogus imp.banner survives with no format, w or h.

An outstream-video-only ad unit is then sent to PBS and every ORTB bidder as banner-eligible with an empty banner object — invalid ORTB, and depending on the exchange either a rejected imp or banner bids returned into a video slot.

Suggest gating on adUnit.mediaTypes.banner existing, and mirroring into mediaTypes.video.pos if video coverage is wanted.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
// OpenRTB ad position (imp.banner.pos), per AdCOM 1.0 Placement Positions:
// 2 = Locked (fixed position), 1 = above the fold, 3 = below the fold.
// Carried via the standard field; ext.adslot.fixed keeps the raw flag too.
const pos = fixed ? 2 : (rect.top < vh ? 1 : 3);

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rect.top is viewport-relative, so pos changes with scroll position: on a refresh auction after the user has scrolled 2000px, a slot genuinely 3000px down the page reports 1 ("above the fold"), and a slot scrolled past (negative top) also reports 1.

"Above the fold" is a document-relative property — whether the slot falls within the page's initial viewport — and the module already computes the right coordinate as y on line 91. (rect.top + scrollY) < vh makes this stable regardless of when the measurement runs.

Two smaller points on the same line: the AdCOM citation is right that 2 was "Locked", but OpenRTB 2.5 §5.4 marks 2 as DEPRECATED, and this value reaches bidders that relay OpenRTB 2.x — consider 1/3 plus the fixed flag in the module's own ext object, or emitting 0 (unknown) rather than guessing. And getWinDimensions().document.documentElement.clientHeight would be consistent with what percentInView compares against.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated

// Visibility ratio (0..1). percentInView is Prebid's shared viewability
// helper and returns a 0..100 percentage.
const ratio = Number((percentInView(el) / 100).toFixed(2));

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

percentInView takes a {w, h} size override, and every other caller in the repo passes it — 33acrossBidAdapter.js:478, marsmediaBidAdapter.js:371, underdogmediaBidAdapter.js:274, omsBidAdapter.js:283, onomagicBidAdapter.js:176, valuadBidAdapter.js:27, libraries/omsUtils/viewability.js:18.

It matters here because measurement happens pre-auction, when a GPT slot is normally still an empty block-level <div> with height 0. percentInViewStaticgetIntersectionOfRects returns null as soon as bbox.top >= bbox.bottom, so percentInView returns 0 and the emitted signal is ratio: 0, h: 0 — for the most common case on the page, on first load.

Suggest passing the ad unit's banner size: percentInView(el, {w, h}), or using getViewability(el, topWin, size) from the same library, which also applies the visibilityState gate.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
};

const afterLayout = () => {
if (typeof window.requestAnimationFrame === 'function') {

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

requestAnimationFrame callbacks are suspended in background and prerendered tabs, so in a hidden tab measureAndDone never runs and callback() is never called. With the config the .md documents (waitForIt: true, auctionDelay: 300), RTD core then has to wait out its own timeout on every auction: 300ms of added latency and zero signals delivered. The readyState === 'loading' path on line 130 can stall the same way.

RTD passes the remaining budget as the 5th argument to getBidRequestData — it's in the type (modules/rtdModule/spec.ts:55) and supplied by modules/rtdModule/index.ts — but the signature here only takes two. Taking timeout and arming a fallback setTimeout that measures (or at least calls callback()) inside the budget fixes both, and a document.visibilityState === 'visible' gate avoids measuring a hidden tab at all.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
logWarn(`[UNICORN RTD] element not found for adUnit "${adUnit.code}" (divId="${divId}")`);
}
});
logInfo('[UNICORN RTD] injected adslot signals');

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This logs "injected adslot signals" unconditionally, including when every ad unit failed to resolve and nothing was injected.

Generated by Claude.

Comment thread modules/unicornRtdProvider.js Outdated
}
}

/** @type {import('../modules/rtdModule/index.js').RtdSubmodule} */

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

RtdSubmodule doesn't exist — there are no occurrences of that name anywhere in modules/rtdModule/. The real type is RtdProviderSpec<P> (modules/rtdModule/spec.ts:42), and because tsconfig.json sets checkJs: false this annotation is a silent no-op that validates nothing. (The missing timeout parameter noted on line 121 is exactly what it would have caught.)

Per CLAUDE.md, new modules should be TypeScript with types for their public interface, and seven recent RTD providers already are (encypher, geolocation, humansecurity, insurads, mile, scope3, stackup). As unicornRtdProvider.ts typed RtdProviderSpec<'unicorn'> the correct getBidRequestData signature comes for free — and the shape of the injected signal object is this module's public interface, so it deserves an exported type.

Generated by Claude.

Comment thread modules/unicornBidAdapter.js Outdated
logInfo('[UNICORN] buildOpenRtbBidRequestPayload.bidderRequest:', bidderRequest);
const imp = validBidRequests.map(br => {
return {
const adslotSignal = deepAccess(br, 'ortb2Imp.ext.data.adslot');

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two things here. This needs to follow the namespace change suggested on modules/unicornRtdProvider.js:109. And if (adslotSignal) on line 67 accepts any truthy value, so a publisher who has set ortb2Imp.ext.data.adslot = '/1234/homepage' for other bidders — the established meaning of that key — makes Unicorn send imp.ext.adslot: "/1234/homepage", a string where the module's own documented schema says an object with ver. Worth guarding with isPlainObject(signal) && signal.ver once the key is namespaced.

Generated by Claude.

Comment thread modules/unicornBidAdapter.js Outdated
bidfloor: parseFloat(0)
};
if (adslotSignal) {
// wire contract: imp.ext.adslot (flat key, received like skadn) — see spec

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This comment describes a negotiation rather than the code — it points at a spec that isn't in this PR, and skadn is unrelated to what's happening on this line. Worth dropping or restating as what the code does.

Generated by Claude.

expect(signal.ratio).to.be.within(0, 1);
// position lives in the standard banner.pos, not inside ext.adslot
expect(signal).to.not.have.property('pos');
expect(au.ortb2Imp.banner.pos).to.be.oneOf([1, 3]);

@dgirardi dgirardi Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These two assertions are the ones that should be pinning the module's core behaviour, but oneOf([1, 3]) and within(0, 1) (line 66) pass for almost any implementation — neither the fold logic nor the ratio computation is actually verified. With a controlled rect and stubbed window dimensions both can be asserted exactly. (That pos couldn't be predicted here is itself a symptom of the window mismatch noted on modules/unicornRtdProvider.js:61 — Karma runs this spec in an iframe.)

Coverage gaps in the same file: every test supplies ortb2Imp.ext.data.divId, so neither the GPT branch nor the adUnit-code fallback in resolveDivId (lines 45-46) is exercised — window.googletag would need stubbing. The readyState === 'loading' / DOMContentLoaded path (130-131) and the no-rAF fallback (123-124) are also untested, which is where the hidden-tab problem lives.

Generated by Claude.

…pter

Per review feedback (patmmccann, dgirardi), moves slot position/geometry/
viewability measurement out of the RTD module and into unicornBidAdapter's
buildRequests. This resolves the structural issues that came from writing to
shared FPD (ortb2Imp), which affected every bidder on the page, not just
Unicorn:

- ext.data.adslot collided with an established key (a GAM slot path string
  read by beopBidAdapter/rubiconBidAdapter) — gone, since imp.ext.unicorn is
  now built only inside this adapter's own OpenRTB payload.
- banner.pos was written unconditionally regardless of mediaType, corrupting
  ortb2Imp for video/native-only ad units — gone, same reason.
- The RTD timeout/hidden-tab/stale-ad-unit-mutation issues (RTD ignored the
  timeout argument, requestAnimationFrame stalls in background tabs, and
  ad unit objects were mutated in place) no longer apply: measurement is a
  single synchronous read per bid request, the same pattern other adapters
  use with percentInView (33across, marsmedia, oms, ...).

Also fixes, in the new inline implementation:
- banner.pos now compares document-relative y (rect + scroll) against
  viewport height, instead of viewport-relative rect.top, so it no longer
  flips after the user scrolls and a refresh auction runs.
- Drops the AdCOM "Locked" pos=2 value — OpenRTB 2.5 marks 2 DEPRECATED, and
  it would have reached bidders that relay OpenRTB. Fixed/sticky is now only
  carried in ext.unicorn's own fixed/sticky flags (kept as two flags, since
  `position: sticky` alone doesn't mean "currently stuck").
- Detects fixed/sticky by walking ancestors, not just the slot element,
  since anchor/sticky ad units are usually a fixed/sticky wrapper around a
  statically positioned ad div.
- percentInView/getViewability are now called with the ad unit's size, so an
  unrendered (0x0) GPT slot still gets a real ratio/w/h via the size
  override, instead of ratio:0, h:0.
- getViewportOffset compensates for measuring from inside a friendly iframe,
  consistent with how percentInView itself handles that case.
- A publisher-declared ortb2Imp.banner.pos, if present, is used instead of
  the measured value.

Removes modules/unicornRtdProvider.{js,md}, its spec, and the
modules/.submodules.json entry. Documents the new imp.banner.pos/imp.ext.unicorn
behavior in modules/unicornBidAdapter.md, with adapter spec coverage for the
fold-vs-scroll fix, the size-override fix, fixed vs sticky detection, GPT slot
mapping, the ortb2Imp.banner.pos override, and the no-matching-element case.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 5, 2026 03:13
@harufujimoto

Copy link
Copy Markdown
Contributor Author

Hi @dgirardi; Thanks for the thorough review — moved the measurement into the bid adapter's buildRequests, as you and @patmmccann both suggested. This removes the structural issues (the ext.data.adslot collision, the ungated banner.pos write, the RTD timeout/hidden-tab handling, and the stale-mutation issue) by construction, since nothing is written to shared ortb2Imp anymore — the signal only ever exists inside this adapter's own OpenRTB payload.

Also fixed in the same pass: pos now uses document-relative y instead of viewport-relative rect.top (stable across scroll), dropped pos=2 (OpenRTB 2.5 deprecated) in favor of separate fixed/sticky flags, walk ancestors to detect fixed/sticky wrappers, pass the ad unit size to percentInView/getViewability so an unrendered slot still gets a real ratio, and compensate via getViewportOffset for friendly-iframe measurement. A publisher-declared ortb2Imp.banner.pos now takes priority over the measurement.

Adapter tests now cover the fold/scroll fix, the size-override fix, fixed-vs-sticky, GPT slot mapping, and the no-matching-element case with exact assertions (stubbed window dims/offset, controlled rects) rather than loose bounds checks.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

modules/unicornBidAdapter.js:90

  • The PR title references an "RTD Provider", but the changes here implement slot measurement inside the Unicorn bid adapter (and docs/tests for that). Consider renaming the PR title (or updating the description) to match the actual scope so reviewers can quickly understand what’s being changed.
 * This runs synchronously in buildRequests, when the auction is already
 * dispatching bid requests to bidders — by that point slot elements are
 * expected to be in the DOM, the same assumption other adapters that call
 * `percentInView` synchronously (33across, marsmedia, oms, ...) rely on.
 * Measuring here — rather than pre-auction in a Real-Time Data submodule —
 * keeps the signal scoped to this adapter's own OpenRTB payload: nothing is
 * written back to `ortb2Imp`, so no other bidder or PBS can be affected by it.
 */

if (adslot) {
// Slot geometry/viewability, sent only in this adapter's own OpenRTB
// payload — not shared FPD, so no other bidder or PBS ever sees it.
impObj.ext = { unicorn: adslot.signal };

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The name clashing is no longer a concern given that this is only seen by your exchange; I don't mind either way but I thought I'd note it in case unicorn means server side work that wouldn't be necessary with adslot.

@patmmccann patmmccann changed the title Unicorn RTD Provider: add slot position & viewability module Unicorn Bid Provider: add slot position & viewability module Aug 8, 2026
@patmmccann
patmmccann merged commit c9b8da6 into prebid:master Aug 8, 2026
114 checks passed
harufujimoto added a commit to bulbit/prebid.github.io.uc that referenced this pull request Aug 12, 2026
The RTD provider was removed upstream (prebid/Prebid.js#15379) — the
position/geometry/viewability measurement now runs inline in the bid adapter.
This makes the standalone RTD module doc obsolete (it also described a stale
wire format: ortb2Imp.ext.data.adslot / imp.ext.adslot / banner.pos=2).

Remove dev-docs/modules/unicornRtdProvider.md and instead document the current,
adapter-native behavior in the bidder doc: imp.banner.pos (1/3) and
imp.ext.unicorn { ver, ratio, fixed, sticky, w, h, x, y }, plus the slot
element resolution order.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
patmmccann pushed a commit that referenced this pull request Aug 12, 2026
…adslot (#15483)

Forwards the Global Placement ID: when the gpid / gptPreAuction module has set
`ortb2Imp.ext.gpid`, the adapter sends it on the wire as `imp.ext.gpid`.

Also renames the position/geometry/viewability signal key from
`imp.ext.unicorn` to `imp.ext.adslot`. Now that this signal lives only in the
adapter's own OpenRTB payload (not shared FPD), the earlier namespace-collision
concern no longer applies, and `adslot` is the more descriptive name for slot
position/geometry — as noted in the review of #15379. Both keys touch
`imp.ext`, so the rename and the gpid addition are done together to keep the
merge of `imp.ext = { adslot, gpid }` in one place.

Adapter spec covers gpid forwarding, gpid omitted when absent, and adslot +
gpid coexisting under imp.ext.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
muuki88 pushed a commit to prebid/prebid.github.io that referenced this pull request Aug 14, 2026
* Unicorn RTD Provider: add module documentation

Docs for the Unicorn RTD Provider submodule added in prebid/Prebid.js#15379.

* Unicorn RTD Provider docs: wrap maintainer email to satisfy markdownlint MD034

* Document UNICORN adapter position signals; drop obsolete RTD module doc

The RTD provider was removed upstream (prebid/Prebid.js#15379) — the
position/geometry/viewability measurement now runs inline in the bid adapter.
This makes the standalone RTD module doc obsolete (it also described a stale
wire format: ortb2Imp.ext.data.adslot / imp.ext.adslot / banner.pos=2).

Remove dev-docs/modules/unicornRtdProvider.md and instead document the current,
adapter-native behavior in the bidder doc: imp.banner.pos (1/3) and
imp.ext.unicorn { ver, ratio, fixed, sticky, w, h, x, y }, plus the slot
element resolution order.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix markdownlint in unicorn bidder doc (heading level, compact table)

Editing the file made CI lint the whole doc, surfacing pre-existing
violations: MD001 (first heading must be h2 given the front-matter title,
not h3) and MD060 (table must use the compact pipe style). Reformat the
heading levels and the bid-params table to match the repo's markdownlint
config; verified locally with markdownlint-cli2 (0 issues).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Restore aligned bid-params table; sync key name to imp.ext.adslot

Per review feedback, keep the readable aligned table instead of the compact
reformat. The original failed markdownlint MD060 only because it was
inconsistent (aligned header, compact data rows); aligning all rows satisfies
MD060's "aligned" style while preserving readability (verified locally with
markdownlint-cli2, 0 issues).

Also update the position-signal key from imp.ext.unicorn to imp.ext.adslot to
match the adapter after prebid/Prebid.js#15483, and document imp.ext.gpid.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@patmmccann patmmccann removed the core label Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants