Open
Conversation
3 tasks
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Contributor
AGENT PR REVIEW TESTINGPR #4706 Review: New Adapter: T Advertising x BlisAuthor: tb-emq | Type: New Adapter | CI: Clean (all 6 checks passed) Triage Manifest
File Routing
Issues FoundFAIL:
|
| Check | Result | Evidence |
|---|---|---|
| URL format | PASS | Well-formed HTTPS URL |
| Reachability | PASS | HTTP 204 on POST, 1.57s response time |
| SSL/TLS | PASS | Amazon RSA 2048 M04 cert (CN=tads.xplosion.de), valid until May 16, 2026 |
| Domain ownership | PASS | tads.xplosion.de — Xplosion is a known emetriq/T-Systems platform. Consistent with maintainer email dev@emetriq.com |
| No hardcoded credentials | PASS | Clean URL |
| No template macros | PASS | No {{...}} present |
endpointCompression: gzip
| Check | Result | Evidence |
|---|---|---|
| Valid value | PASS | gzip is the only accepted value |
geoscope: [global]
| Check | Result | Evidence |
|---|---|---|
| Valid value | PASS | global is a recognized geoscope value (CI validates and passes) |
| Casing | INFO | Convention in field-index recommends uppercase (GLOBAL), though CI passes with lowercase |
maintainer.email: "dev@emetriq.com"
| Check | Result | Evidence |
|---|---|---|
| Email format | PASS | Valid format user@domain.tld |
| Domain consistency | PASS | emetriq.com — emetriq is the parent company behind T Advertising, consistent with PR author tb-emq |
gvlVendorID: 213
| Check | Result | Evidence |
|---|---|---|
| Value range | PASS | Positive integer |
| Company match | PASS | GVL vendor 213 corresponds to emetriq GmbH, which is the company behind T Advertising/TADS |
modifyingVastXmlAllowed: true
| Check | Result | Evidence |
|---|---|---|
| Relevance | PASS | Adapter supports video media type — setting is appropriate |
openrtb.version: "2.6"
| Check | Result | Evidence |
|---|---|---|
| Valid value | PASS | "2.6" is a recognized OpenRTB version |
| Code compatibility | PASS | Adapter uses bid.MType (OpenRTB 2.6 feature) for media type resolution |
capabilities: site (banner, video)
| Check | Result | Evidence |
|---|---|---|
| Valid media types | PASS | banner and video are valid |
| At least one per platform | PASS | site has 2 media types |
No app platform |
PASS | Adapter code only handles request.Site, consistent with site-only capabilities |
| Native mismatch | FAIL | Adapter code handles native (MType 4) and has native test fixture, but native not declared in capabilities |
Bidder-Params: tadvertising_blis.json + imp_tadvertisingblis.go + params_test.go
JSON Schema
| Check | Result | Evidence |
|---|---|---|
| Draft-04 schema | PASS | $schema declares draft-04 |
| Required fields | PASS | publisherId is required |
| Property type | PASS | type: string, minLength: 1, maxLength: 32 — appropriate constraints |
| Title format | PASS | "T-Advertising-Blis Adapter Params" follows convention |
| Description present | PASS | "Unique ID of the publisher/supply partner provided by T-Advertising" |
imp_tadvertisingblis.go
| Check | Result | Evidence |
|---|---|---|
| Struct matches schema | PASS | PublisherId string maps to schema publisherId |
| No omitempty on required fields | PASS | json:"publisherId" has no omitempty — correct for a required field |
| json tag matches schema | PASS | json:"publisherId" matches schema property name exactly |
| Package is openrtb_ext | PASS | package openrtb_ext |
| Type is exported | PASS | ImpExtTadvertising starts with uppercase |
params_test.go
| Check | Result | Evidence |
|---|---|---|
| Valid params tested | PASS | 1 valid case: {"publisherId": "1427ab10f2e448057ed3b422"} |
| Invalid params tested | PASS | 4 cases: empty object, empty string, wrong type (numeric), too long (33 chars) |
| Uses correct bidder constant | PASS | openrtb_ext.BidderTAdvertisingBlis |
| Correct schema path | PASS | "../../static/bidder-params" |
| Missing null test | INFO | No explicit null test case, though this is minor since the empty object case covers the required field check |
| Test function names | PASS | TestValidParams and TestInvalidParams |
Adapter-Code: tadvertising_blis.go + tests + registration
Builder Function
| Check | Result | Evidence |
|---|---|---|
| Correct signature | PASS | Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server config.Server) (adapters.Bidder, error) |
| Endpoint from config | PASS | Uses config.Endpoint |
| No hardcoded credentials | PASS | No secrets in builder |
| No template macros | PASS | Endpoint used directly, no macro resolution needed |
MakeRequests
| Check | Result | Evidence |
|---|---|---|
| Imp ext unmarshaling | PASS | Two-step: adapters.ExtImpBidder → openrtb_ext.ImpExtTadvertising via jsonutil.Unmarshal |
| Uses shared imp ext struct | PASS | Uses openrtb_ext.ImpExtTadvertising from openrtb_ext/imp_tadvertisingblis.go |
| ImpIDs present | PASS | openrtb_ext.GetImpIDs(request.Imp) called |
| Headers | PASS | Content-Type: application/json + custom X-Supply-Partner-Id header |
| Copy semantics (Site) | PASS | Creates siteCopy := *request.Site before mutation |
| Copy semantics (Publisher) | PASS | Creates sitePublisherCopy := *request.Site.Publisher before mutation |
| Marshal error handled | PASS | Returns error from jsonutil.Marshal |
| Uses jsonutil | PASS | jsonutil.Marshal and jsonutil.Unmarshal throughout |
| Method constant | WARN | Uses "POST" string literal instead of http.MethodPost |
| Error types | PASS | Uses errortypes.BadInput for invalid request data |
| Multi-imp handling | PASS | Batches all impressions in a single request. Reads only Imp[0].Ext for publisherId — acceptable since all imps share the same publisher context |
MakeBids
| Check | Result | Evidence |
|---|---|---|
| HTTP status handling | PASS | 204 → nil, nil; error check before unmarshal |
| Response unmarshaling | PASS | jsonutil.Unmarshal used |
| Bid type resolution | PASS | Uses bid.MType switch (banner/video/native) |
| BidderResponse construction | PASS | adapters.NewBidderResponseWithBidsCapacity(len(request.Imp)) |
| Bid pointer safety | PASS | Uses iterutil.SlicePointerValues — provides correct pointer iteration |
| Error return | FAIL | return bidResponse, nil — accumulated errors never returned. Should be return bidResponse, errors |
| Currency handling | WARN | bidResponse.Currency = response.Cur set unconditionally — empty Cur overwrites default USD |
Macro Resolution (resolveMacros)
| Check | Result | Evidence |
|---|---|---|
Replaces ${AUCTION_PRICE} in NURL |
PASS | strings.ReplaceAll(bid.NURL, ...) |
Replaces ${AUCTION_PRICE} in AdM |
PASS | strings.ReplaceAll(bid.AdM, ...) |
Replaces ${AUCTION_PRICE} in BURL |
PASS | strings.ReplaceAll(bid.BURL, ...) |
| Null check | PASS | if bid != nil guard |
| No JSON injection risk | PASS | String replacement on plain text fields, not raw JSON |
Registration
| Check | Result | Evidence |
|---|---|---|
openrtb_ext/bidders.go |
PASS | BidderTAdvertisingBlis constant added |
exchange/adapter_builders.go |
PASS | Import and builder map entry added |
Test Runner
| Check | Result | Evidence |
|---|---|---|
| Package name | PASS | package tadvertising_blis |
| Test function | PASS | func TestJsonSamples(t *testing.T) |
| Builder invocation | PASS | Correct bidder constant, fake endpoint, server config |
| Test directory | PASS | "tadvertising_blistest" — correct naming |
| Build error check | PASS | require.NoError(t, buildErr) |
| Minimal test runner | PASS | ~20 lines, thin wrapper |
Test Data
| Check | Result | Evidence |
|---|---|---|
| Exemplary fixtures | PASS | 3 files: banner.json, video.json, native.json |
| Supplemental fixtures | PASS | 8 files: edge cases, error paths, macro replacement, multi-imp |
| Banner coverage | PASS | exemplary/banner.json with MType 1 |
| Video coverage | PASS | exemplary/video.json with MType 2, includes VAST markup |
| Native coverage | FAIL | exemplary/native.json exists but native not in capabilities |
| Macro replacement coverage | PASS | supplemental/bid-macro-replace.json tests ${AUCTION_PRICE} in NURL, AdM, BURL |
| Error path: invalid ext | PASS | invalid-imp-ext.json, invalid-imp-ext-bidder.json |
| Error path: HTTP 204 | PASS | response-http-204.json |
| Error path: HTTP 500 | PASS | response-http-500.json |
| Error path: unsupported media type | WARN | Present but missing expectedMakeBidsErrors (masked by the error return bug) |
| Edge case: no site | PASS | banner_no_site.json — adapter creates Site+Publisher from scratch |
| Edge case: no publisher | PASS | banner_no_publisher.json — adapter creates Publisher |
| Edge case: no publisher ID | PASS | banner_no_publisher_id.json |
| Multi-impression | PASS | multiImpressionsRequest.json — 2 impressions, 2 bids returned |
| App context test | N/A | App not declared in capabilities — not required |
PR-Level Checks
| Check | Result | Evidence |
|---|---|---|
| Documentation PR | PASS | PR description links prebid/prebid.github.io#6458 |
| Duplicate PR check | PASS | No other open PRs for "tadvertising_blis" |
| New adapter completeness | PASS | All 10 required file types present |
| CI status | PASS | All 6 checks passed |
| Naming consistency | PASS | Directory tadvertising_blis/, package tadvertising_blis, test dir tadvertising_blistest, constant BidderTAdvertisingBlis — all consistent |
Recommendation: Request Changes
This is a well-structured new adapter PR with good test coverage and clean code patterns. The adapter correctly uses jsonutil, iterutil.SlicePointerValues, copy semantics for Site/Publisher, and MType-based bid type resolution. However, there are two issues that must be fixed:
- Must fix:
MakeBidsreturnsnilinstead of accumulated errors — changereturn bidResponse, niltoreturn bidResponse, errors. Then updateunsupported-media-type.jsonto includeexpectedMakeBidsErrors. - Must fix: Capabilities declare only
banner+video, but the adapter handlesnativeand has anative.jsonexemplary test. Either addnativeto capabilities or remove the dead native handling. - Should fix: Use
http.MethodPostinstead of string literal"POST" - Should fix: Guard currency assignment:
if response.Cur != "" { bidResponse.Currency = response.Cur } - Consider: Capitalize geoscope to
GLOBALper convention
Task Completion Summary
| Task # | Subject | Status |
|---|---|---|
| 1 | PR Triage: Classify PR type, route files, check CI | Completed |
| 2 | Bidder-info: Verify tadvertising_blis.yaml fields | Completed |
| 3 | Bidder-params: Verify JSON schema + imp struct + params_test | Completed |
| 4 | Adapter-code: Verify adapter Go code, MakeRequests, MakeBids | Completed |
| 5 | Adapter-code: Verify test runner, test data, registration | Completed |
| 6 | Produce final review summary for PR #4706 | Completed |
Skills Called
| Skill | Outcome |
|---|---|
pr-triage |
Executed inline — file categorization, CI check (all 6 passed), completeness check (all 10 files present), cross-skill concerns (native capability mismatch) |
bidder-info-pr-review |
Executed inline — endpoint reachability (204), SSL (Amazon RSA, valid May 2026), GVL (213 = emetriq), capabilities, geoscope |
bidder-params-pr-review |
Executed inline — schema validation (draft-04, required publisherId), struct alignment, params_test coverage |
adapter-code-pr-review |
Executed inline — code quality (jsonutil, copy semantics, ImpIDs), error handling bug found, registration, test data coverage |
Code / Commands Executed
| Command | Purpose | Result |
|---|---|---|
curl -X POST https://prebid-blis.tads.xplosion.de/bid |
Endpoint reachability | HTTP 204, 1.57s |
openssl s_client -connect prebid-blis.tads.xplosion.de:443 |
SSL certificate details | Amazon RSA 2048 M04, CN=tads.xplosion.de, valid until May 16 2026 |
WebFetch API Calls
| URL | Purpose |
|---|---|
api.github.com/.../pulls/4706 |
PR metadata (title, author, description) |
api.github.com/.../pulls/4706/files |
File list + patches |
api.github.com/.../pulls/4706/commits |
Head SHA for CI check |
api.github.com/.../commits/{sha}/check-runs |
CI status (all 6 passed) |
raw.githubusercontent.com/.../tadvertising_blis.go |
Full adapter source code |
raw.githubusercontent.com/.../tadvertising_blis.yaml |
Bidder-info YAML |
raw.githubusercontent.com/.../tadvertising_blis.json |
Params JSON schema |
raw.githubusercontent.com/.../imp_tadvertisingblis.go |
Imp ext struct |
raw.githubusercontent.com/.../params_test.go |
Params validation tests |
raw.githubusercontent.com/.../tadvertising_blis_test.go |
Test runner |
raw.githubusercontent.com/.../exemplary/banner.json |
Banner test fixture |
raw.githubusercontent.com/.../exemplary/video.json |
Video test fixture |
raw.githubusercontent.com/.../exemplary/native.json |
Native test fixture |
raw.githubusercontent.com/.../supplemental/bid-macro-replace.json |
Macro replacement test |
raw.githubusercontent.com/.../supplemental/unsupported-media-type.json |
Unsupported media type test |
raw.githubusercontent.com/.../supplemental/multiImpressionsRequest.json |
Multi-impression test |
vendor-list.consensu.org/v3/vendor-list.json |
GVL vendor lookup (vendor 213) |
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
…tising-blis # Conflicts: # exchange/adapter_builders.go
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
linux019
reviewed
Mar 16, 2026
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
linux019
reviewed
Mar 23, 2026
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Contributor
postindustria-code
left a comment
postindustria-code
left a comment
There was a problem hiding this comment.
Does T-Advertising Blis intend to support user sync?
The adapter has site capabilities and it could be beneficial for it.
Code coverage summaryNote:
tadvertising_blisRefer here for heat map coverage report |
Author
At the current time we do not intent to support user syncs. This may be revisited in the future as the business needs change down the line but it is not a current priority. |
postindustria-code
approved these changes
Mar 30, 2026
Contributor
postindustria-code
left a comment
postindustria-code
left a comment
There was a problem hiding this comment.
Thanks for the clarification on the user sync.
Overall, the PR is well-structured and looks solid. LGTM.
linux019
approved these changes
Apr 7, 2026
Author
|
Dear Maintainers, Thanks for your time and the thorough reviews. |
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
5 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.
Doc PR: prebid/prebid.github.io#6458
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
New Bidder Adapter for T Advertising Solutions in cooperation with Blis