Show requested, fill, and observed slot sizes in GPT diagnostics

[ChristianPavilonis]

Summary

• retain each correlated Trusted Server configured slot-size list
• retain GPT slotRenderEnded.size as the reported fill size
• retain the measured outer slot box to explain flexible/APS 1×1 fills
• display and export all three metrics, with stale-cycle guards and SafeFrame limitations documented

Closes #1031

Validation

• cd crates/trusted-server-js/lib && npx vitest run (45 files, 838 tests)
• focused GPT/diagnostics tests (283 passed)
• cd crates/trusted-server-js/lib && npm run format
• git diff --check

[prk-Jr]

Summary

Separating the configured request list, GPT's reported fill size, and a measured outer box is the right decomposition for diagnosing flexible/APS 1×1 fills, and the stale-cycle guard is designed well. Two things need a change before merge: on publisher-defined slots the new requestedSlotSizes reports a size list GPT never received, and the new observer's window type permits a call that throws inside a ResizeObserver callback.

Blocking

🔧 wrench

• requestedSlotSizes records sizes GPT never received: slot.formats is passed unconditionally, but TS only supplies formats on the defineSlot path; the existingSlot (publisher-defined) path never applies sizes to the slot, and there is no defineSizeMapping/setSizes in the integration (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1061).
• instanceof this.window.HTMLElement throws for a type-valid window: SlotSizeWindow does not declare HTMLElement; tsc reports it and an injected { ResizeObserver } window throws uncaught inside the observer callback. Unreachable via installGptDiagnosticsRuntime, so reasonable to downgrade (crates/trusted-server-js/lib/src/integrations/gpt_diagnostics/slot_size_observer.ts:105).

Non-blocking

♻️ refactor

• Round the measured box: sub-pixel rects are stored and displayed verbatim, and each delta re-notifies (slot_size_observer.ts:142).
• Badge line can carry 16 sizes inside a 260px badge (badges.ts:123).
• Extend the export type gate: test/integrations/gpt_diagnostics/types.test.ts is the only expectTypeOf gate on the export allowlist and says to keep additions synchronized, but neither requestedSlotSizes nor observedSlotSize got an assertion there. api.test.ts covers the runtime key list only.

🤔 thinking

• 0×0 observed boxes are recorded for collapsed/hidden slots — likely desirable, worth documenting as deliberate (store.ts:712).

⛏ nitpick

• Duplicated size formatting between badges.ts:109 and overlay.ts:282.
• defaultScheduleFrame ignores the injected window (slot_size_observer.ts:31), consistent with the existing pattern in binding.ts/badges.ts.

👍 praise

• Stale-cycle guard ownership split between observer and store, tested from both sides (store.ts:706).
• normalizedRequestedSlotSizes freezes and copies, with a test that mutates the caller's formats array after recording.
• Collapsing the arity-branching recorder call in gpt/index.ts into a single call is a clean simplification.

CI Status

All 19 GitHub checks pass on f5ca22e. Re-run locally against the PR head:

• vitest (gpt + gpt_diagnostics): PASS (16 files, 359 tests)
• eslint (changed areas): PASS
• prettier: PASS
• tsc --noEmit: pre-existing baseline failure (170 errors on main), not a gate; the three new errors in slot_size_observer.ts are the 🔧 above.

[prk-Jr]

🔧 wrench — requestedSlotSizes records sizes GPT never received on publisher-defined slots.

slot.formats is passed unconditionally, but Trusted Server only supplies formats to GPT on the defineSlot path above (line 1023). When existingSlot is found — the publisher defined the div first — TS reuses that slot and never applies sizes to it; there is no defineSizeMapping/setSizes anywhere in the integration. On those slots the panel shows Requested slot sizes 300×250, 728×90 for a list GPT never saw, and the docs assert it is "the configured AuctionSlot.formats list Trusted Server supplied to GPT for that request". That is the exact misdirection #1031 exists to remove.

Note tsOwned ? slot.formats : undefined is not the fix either: refresh cycles re-find TS's own slot through getSlots(), so tsOwned is false there and the accurate list would be dropped. Keying on the handoff record survives refreshes:

const tsDefinedFormats = ts.gptSlotHandoffs?.[actualDivId]?.formats;
ts.gptDiagnosticsRecorder?.recordTrustedServerOpportunity(
  gptSlot,
  slot.id,
  opportunity,
  bid.hb_auction_id,
  tsDefinedFormats
);

Alternatively keep recording it but relabel the field, badge, panel, and docs as Trusted Server's configured sizes rather than what GPT was given.

[prk-Jr]

🔧 wrench — instanceof this.window.HTMLElement throws for a window that satisfies the declared type.

SlotSizeWindow (line 17) never declares HTMLElement, and lib.dom's Window does not carry it — only Window & typeof globalThis does. Injecting { ResizeObserver } throws TypeError: Right-hand side of 'instanceof' is not an object, uncaught, from inside the ResizeObserver callback, which contradicts the module's "diagnostics never interrupt the page" invariant. tsc already reports three errors here (105–107). binding.ts gets this right by declaring HTMLElement: typeof HTMLElement on its window type.

Not reachable through installGptDiagnosticsRuntime, which passes a real window, so downgrade this if you prefer — but the fix is one line:

type SlotSizeWindow = Window & {
  HTMLElement: typeof HTMLElement;
  ResizeObserver?: typeof ResizeObserver;
};

[prk-Jr]

♻️ refactor — round the measured box.

getBoundingClientRect() returns sub-pixel values and nothing rounds them, so the store retains e.g. [728.328125, 90.5] verbatim and the badge reads Outer box 728.328125×90.5 next to a clean GPT 1×1. Every sub-pixel delta also clears the equality guard in recordObservedSlotSize, so it notifies, re-renders badges and panel, and schedules another observer refresh. Math.round on both dimensions fixes the display and the churn:

this.store.recordObservedSlotSize(cycle.runtimeSlotNumber, cycle.requestNumber, [
  Math.round(rectangle.width),
  Math.round(rectangle.height),
]);

[prk-Jr]

⛏ nitpick — defaultScheduleFrame uses the global requestAnimationFrame rather than the injected window's, so an option-provided window is ignored for frame scheduling. Same wart already exists in binding.ts and badges.ts, so this is consistency, not a regression.

[prk-Jr]

♻️ refactor — the badge line can carry up to 16 sizes.

MAX_REQUESTED_SLOT_SIZES is 16, and formatSizes joins all of them into a badge capped at BADGE_MAX_WIDTH_PX (260px), so a multi-format slot wraps a long Requested … string over the creative. Consider truncating the badge to the first two or three plus +N more, and keeping the complete list in the panel, which already has room for it.

[prk-Jr]

⛏ nitpick — formatSizes here duplicates the inline .map(...).join(', ') in overlay.ts (cycleFacts). One shared helper keeps the two surfaces from drifting in separator or × glyph.

[prk-Jr]

🤔 thinking — the validator accepts 0 in both dimensions, so a breakpoint-hidden duplicate slot records Outer box 0×0. That may be the most useful signal available for those slots (it tells the operator the element is collapsed rather than mis-sized), so this is not a request to change it — just worth making the choice deliberate and saying so in the docs alongside the other rejection rules.

[prk-Jr]

👍 praise — the stale-cycle split is the right shape: the observer only forwards the (runtimeSlotNumber, requestNumber) identity captured at schedule time, and the store owns rejection by re-checking latest-cycle, filled, and render-observed. Both sides are covered by tests, and the delayed-measurement case is asserted against a real refresh rather than mocked away.

[aram356]

Summary

Adds three separately-labelled size facts to GPT diagnostics — configured requestedSlotSizes, the GPT-reported fill size, and a new observedSlotSize measured via GptDiagnosticsSlotSizeObserver. The design is solid: validation is thorough, the stale-cycle rejection lives at the store level (defense-in-depth), and the new paths are well tested. Requesting changes for two 🔧 items: new strict-mode type errors introduced in the new code, and unrounded sub-pixel measurements flowing into the badge/overlay/export.

Blocking

🔧 wrench

• New strict-mode type errors in new code: SlotSizeWindow omits HTMLElement (slot_size_observer.ts:17, errors at 105–107) and record is possibly undefined in recordObservedSlotSize (store.ts:722). Both are new relative to main; binding.ts already models the window-type fix.
• Fractional observed sizes: getBoundingClientRect() values are recorded and displayed unrounded (slot_size_observer.ts:141) — round at the measurement site.

Non-blocking

♻️ refactor

• measure() lacks a destroyed guard (slot_size_observer.ts:126) — currently safe only via destroy order in index.ts.
• Badge first line can explode with up to 16 requested sizes (badges.ts:122) — cap the badge display, keep the full list in the panel/export.

🤔 thinking

• ResizeObserver torn down and rebuilt on every notification (slot_size_observer.ts:98) — convergent but churny; fine to defer.
• Hidden containers record Outer box 0×0 (slot_size_observer.ts:132) — document if intended, otherwise drop zero boxes.

🏕 camp site

• Third copy of defaultScheduleFrame: binding.ts, badges.ts, and now slot_size_observer.ts each carry an identical helper, and size-list formatting is now duplicated between badges.ts (formatSizes) and overlay.ts (inline map/join). A tiny shared helper module would leave the area cleaner.

🌱 seedling

• Test gaps: no test covers the ResizeObserver-unavailable path (which the docs explicitly describe: "otherwise it is the most recently sampled box"), and none covers fractional rect values — which is how the rounding issue slipped through.

⛏ nitpick

• Docs overstate the requested-sizes cap (gpt-diagnostics.md:391) — implementation slices to 16 entries before validating; also a repeated sentence at lines 300–301.

CI Status

• fmt: PASS
• clippy: PASS
• rust tests: PASS (fastly, axum, cloudflare, spin, parity, CLI)
• js tests: PASS (vitest; focused diagnostics + ad_init suites also re-run locally: 283 passed)
• format (typescript/docs): PASS

[aram356]

🔧 wrench — SlotSizeWindow omits HTMLElement, so element instanceof this.window.HTMLElement (line 105) does not typecheck under strict mode, and the Element → HTMLElement narrowing at lines 106–107 fails with it. tsc --noEmit is not a CI gate, but these errors are new relative to main, and the sibling binding.ts:10-14 already solves this exact problem by declaring the property on its window type.

Fix:

type SlotSizeWindow = Window & {
  HTMLElement: typeof HTMLElement;
  ResizeObserver?: typeof ResizeObserver;
};

[aram356]

🔧 wrench — New strict-mode type error (TS18048: 'record' is possibly 'undefined' at line 722): const cycle = record?.requests.find(...) followed by record.requests[record.requests.length - 1] inside the same condition. TypeScript cannot correlate cycle being truthy with record being defined. Baseline main produces no tsc errors in this file; this PR introduces one.

Fix — narrow record first:

const record = this.slots.get(runtimeSlotNumber);
if (!record) return;
const cycle = record.requests.find((candidate) => candidate.requestNumber === requestNumber);
if (
  !cycle ||
  record.requests[record.requests.length - 1] !== cycle ||
  cycle.isEmpty !== false ||
  cycle.renderAtMs === undefined
) {
  return;
}

[aram356]

🔧 wrench — getBoundingClientRect() returns fractional CSS pixels (page zoom, transforms, responsive layouts), and the value flows unrounded into the store, the badge (Outer box 383.99667358398438×90), the overlay, and the JSON export. The tests only exercise integer mocks, so this never surfaces. Rounding here also stabilizes the store's change-dedupe guard against sub-pixel jitter, which otherwise re-notifies and re-runs the whole refresh pipeline on every fraction-of-a-pixel resize.

Fix:

this.store.recordObservedSlotSize(cycle.runtimeSlotNumber, cycle.requestNumber, [
  Math.round(rectangle.width),
  Math.round(rectangle.height),
]);

[aram356]

♻️ refactor — measure() has no destroyed guard, so pending scheduleFrame measure callbacks still run after destroy(). Today this is harmless only because installGptDiagnosticsRuntime destroys bindings in the same synchronous sequence, making bindings.get() return the unbound default — that is coupling to destroy order in index.ts. refresh() and scheduleRefresh() both carry the guard; measure() is the odd one out.

Fix: add if (this.destroyed) return; as the first statement.

[aram356]

♻️ refactor — MAX_REQUESTED_SLOT_SIZES is 16 and the badge renders them all on the first line inside a 260px-max-width badge, so a many-format responsive slot produces a badge that wraps into a wall of text occluding the page. Suggest capping the badge display and leaving the full list to the overlay panel and export:

const shown = cycle.requestedSlotSizes.slice(0, 3);
const extra = cycle.requestedSlotSizes.length - shown.length;
firstLine.push(`Requested ${formatSizes(shown)}${extra > 0 ? ` +${extra}` : ''}`);

[aram356]

🤔 thinking — Every store/bindings notification tears down and re-creates the ResizeObserver and re-observes every bound element — and per spec each observe() fires an initial callback, so every notification schedules a redundant measure per slot on top of the direct scheduleMeasure below. The no-change guard in recordObservedSlotSize keeps this convergent, so it is churn rather than a bug, but a single long-lived observer with set-diffing would do less layout work per notification. Fine to defer.

[aram356]

🤔 thinking — A filled render whose container is display:none or collapsed measures 0×0 here, and both validation layers deliberately allow 0 (< 0 checks only), so the badge shows Outer box 0×0. If "0×0 = collapsed slot evidence" is intended, a sentence in the docs would keep operators from reading it as a measurement bug; if not, drop zero boxes.

[aram356]

⛏ nitpick — "16 valid positive sizes" overstates the cap: normalizedRequestedSlotSizes slices to 16 entries before validating, so invalid entries inside the first 16 shrink the retained count (the store test itself asserts 14). Either reword to "first 16 configured entries, invalid entries dropped" or change the implementation to filter-then-cap.

Also, lines 300–301 repeat themselves: "prevent diagnostics from inspecting iframe content. It does not inspect iframe content…" — worth merging into one sentence.