perf: shrink cached Pokemon 800→352 bytes (field packing, plain-struct scan history)

[TurtIeSocks]

Shrinks the cached Pokemon struct from 800 bytes to 352, moving it from Go's 896-byte allocator size class to the 352-byte class — 544 fewer bytes per cached pokemon. PokemonLookup, the per-candidate scan struct, also drops 18 → 16 bytes. Measured on production instance under live traffic:

	main	this branch
pm2 RSS	15 GB	9.2–9.3 GB (−38%)
GC mark CPU	24.22%	~19%
live heap	6.66 GB	~4.2 GB
pokemon	896 B/obj	352 B/obj
spawnpoint	144 B/obj	112 B/obj

String interning was split out to #395 at @jfberry's request, so it is no longer part of this change.

Why 512 bytes

Go's GC treats objects above 512 bytes materially worse. Measured across 5M live entries with forced GC, mean of 5 runs:

object shape	GC mark
800 B, 12 pointers (the original Pokemon)	279.7 ms
800 B, 1 pointer	271.6 ms
800 B, 0 pointers	62.4 ms
512 B, 1 pointer	62.6 ms
520 B, 1 pointer	194.4 ms
256 B, 1 pointer	33.2 ms

Dropping pointer count from 12 to 1 buys 3%. Adding 8 bytes across the 512 boundary costs 3.1x. TestPokemonUnderGCThreshold guards the line.

Approach

Nullable fields use null.Value[T]. guregu/null's non-generic types are 16 bytes each because they embed sql.NullInt64, and most nullable pokemon columns are tinyint or smallint. null.Value[T] embeds stdlib sql.Null[T] and measures 2/4/8/16/8/2 bytes for uint8/uint16/uint32/uint64/float32/bool. Scan range-rejects and encoding/json renders float32 at 32-bit precision, so both behaviours an earlier draft hand-rolled come from the library.

cell_id and spawn_id are null.Value[int64]. sql.Null[uint64] cannot scan a negative int64 (cell_id is a signed bigint whose real values are frequently negative), and its Value() rejects any uint64 with the high bit set.

Scan history is a plain Go struct. Pokemon embedded grpc.PokemonInternal, a generated protobuf message used purely as in-memory Ditto state, carrying MessageState/SizeCache/UnknownFields at 64 bytes per pokemon and 88 per scan — and only marshalled behind four ANDed conditions with the controlling flag defaulting off. Now []*pokemonScan, 24-byte header and 44 bytes per entry, with protobuf built on demand at the two DB boundaries. Elements stay behind pointers deliberately: the Ditto code holds a pointer to a history entry across other work and mutates through it, so a value slice would have &s[i] invalidated by append.

Fields ordered by descending alignment, and three deleted: Capture1/2/3 (setters with no callers, in neither the select columns nor the upsert), changedFields (const-folded dead in production builds, but its 24-byte header and one GC-scanned word stayed), and seen_type as a string (an eight-value enum in a 24-byte null.String plus a heap pointer, now a one-byte code).

The debug change accumulator rolled out to every entity. Four crossed an allocator class — Station, Spawnpoint, Incident, Tappable. Pokestop, Gym, Route and Player shed 24 bytes but stayed in the same class, so they save nothing at allocation time.

What the profile shows

The first version of this description estimated the CPU saving at "low single-digit percent," sourced from docs/decode-performance-findings.md. That 5.4% figure is from the protobench harness, not production, and was wrong by roughly 5x. @jfberry's "absolutely will need to be benchmarked" was the right call.

Measured on main: GC mark 24.22% of CPU, live heap 6.66 GB against ~14 GB RSS — so about half the process was collector headroom, with GOGC unset. Pokemon were 2.71 GB of that, 40.8%, at ~3.25M cached.

On this branch, per-entity bytes match the predicted size-class ratios to three decimals:

pokemon      896.0 -> 352.0 B/obj   ratio 0.393   size-class predicts 0.393
spawnpoint   144.0 -> 112.0 B/obj   ratio 0.778   size-class predicts 0.778
scan entry    55.8 ->  25.4 B/obj   ratio 0.455   size-class predicts 0.500

Scan entries beat prediction because they also lost their protobuf machinery, not just a size class.

The RSS win outran the struct arithmetic by 1.6x. The model said 1.84 GB of live heap saved, doubled for GOGC=100 headroom, so ~3.7 GB of RSS. pm2 shows 5.75 GB. The difference is second-order and the model did not include it: fewer and more uniform objects mean less GC metadata and less span fragmentation, and HeapReleased went from 8 MB to 647 MB — the runtime is now returning pages where before it essentially was not.

Caveats on the measurement

The two captures were taken at different points in the day. main at 19:17 (evening peak, 3.25M pokemon), this branch at 07:40 (morning trough, 2.74M). That gap is diurnal, not cache warmth. Adjusting for 0.51M more pokemon at 352 B with the GOGC doubling puts the branch near 9.6 GB at peak — still ~5.4 GB and 36% below baseline, but a capture at ~19:00 would settle it properly.

The GC figure is two samples, 18.56% and 19.58%, against a baseline that was a single 30s sample. Read it as ~19% ± 0.5 and ~5 points saved, not 5.7.

The capture is ~25 commits behind. Every production number here was taken on this branch at bca1669. Four review rounds have landed since, all of them correctness fixes rather than size changes, so Pokemon is still 352 — but the figures have not been re-observed.

Nothing has been measured on go 1.27, whose allocator changes could move all of this. Raised by @jfberry and still open.

Wire changes

API responses: weight, height and iv render with fewer digits — 6.7 rather than 6.699999809265137. They originate as protobuf float fields promoted through float64(), so the extra digits were an artifact of that promotion. Verified byte-identical across 500,018 values through both the stdlib and goccy encoders.

Webhook payloads: same fields, same change.

The OpenAPI schema changed on 19 fields, and separately on four timestamps — 16 integers format: int64 → int32, 3 floats double → float. Worth calling out separately: narrowing to unsigned Go types also made huma emit minimum: 0 on those 16 fields. That is a validation constraint rather than a format label, so a strict client or validating proxy would begin rejecting negatives. Accurate to the domain — none of those fields can legitimately be negative — but it is new contract surface beyond the format change, and it was not part of the original ask. TestApiPokemonResultSchemaWidths now pins the shape so a future width change cannot drift it silently.

A later round then took the four timestamp fields the other way. expire_timestamp and updated had become format: int32, which overflows generated clients in 2038 — and inconsistently, since first_seen_timestamp stayed int64. All four are now format: int64 with minimum: 0. Note the second half of that: first_seen_timestamp and changed gain a floor they never advertised, because huma had nothing to infer one from at int64. Accurate to the domain, but new constraint surface.

seen_type enum strings and the golbat_internal protobuf are unchanged.

Review feedback

Round 1 — null already supports generics (the decoder/nulltypes package an earlier draft added is deleted, −702 lines); pull the SeenTypeCodeWild = 0 fix in (code 0 is now an explicit Unset sentinel); the webhook re-widening is wasted (gone); the embedded protobuf is ripe for change (done, stacked here); rename the change list and roll it out (done).

Round 2 — four asks, all addressed:

1. SetSeenType takes a SeenTypeCode, not a string. The constants are typed, so a typo now fails to compile instead of silently no-opping, and the decode path loses a map lookup.
2. The widen layer is gone. widenPtr/widenFloatPtr deleted across 19 call sites by narrowing the response types.
3. Clamp comparisons converge. This was a real bug and the most serious thing in the round — see below.
4. NullSeenType.Scan degrades instead of failing. An unrecognised enum value now warns and yields invalid rather than erroring, because an erroring Scan stranded a newRecord cache entry that re-failed its DB load on every sighting, silently dropping all processing for that pokemon for the duration of a mixed deployment. Value() stays strict — writing an out-of-enum string to a MariaDB ENUM is silently stored as ''.

On #3, since it was introduced by this PR

Several sites compared a clamp-saturated stored value against a raw proto value. With a costume of 300, the stored value saturates at 255 and 255 != 300 on every sighting, forever. The branch that guards nulls out weight, height, size, moves, cp, shiny, ditto and pvp — so every encounter's enrichment was undone by the next sighting, fleet-wide.

An earlier review on this branch flagged it as Minor and "not reachable with today's proto values," and it shipped on that basis. That was wrong twice over: pogo enums are open int32, so unknown and negative wire values pass straight through, and Golbat ingests raw protos from third-party scanners — a malformed packet is enough.

Fixed at 7 sites across 5 functions (3 of which the ask did not name) with non-counting narrowUint8/16/32 helpers on comparison sides, the counting clampUint* delegating to them so golbat_field_clamped_total still fires exactly once per real store. Regression tests cover each site individually — reverting one fails only its own test.

Round 4 — four asks plus a ranked list, all addressed. Two of the four turned out deeper than stated:

• An unrecognised seen_type was degrading to Unset, which is an active state. updateFromWild downgraded the record, updateFromNearby replaced precise coordinates with cell centres, and the save wrote the damage back over a newer binary's value. Pre-PR main stored the string opaquely and every switch fell through harmlessly, so the degrade shipped worse than what it replaced — in exactly the mixed-deployment case it was built for. Now a distinct unknown code, inert at all 17 switch sites, with COALESCE on both write paths so it refuses to overwrite.
• calculateIv computed Iv from the raw IV sum while storing clamped values. iv is float(5,2) unsigned, so a sum above 450 produces a value MariaDB rejects under STRICT_TRANS_TABLES — failing the entire multi-row batch upsert. Narrowing the inputs as specified did not close it, because the clamp used the column's 255 rather than the game's 15. Clamping at 15 does, and subsumes it: sum ≤ 45 means Iv ≤ 100.

Chasing that second one surfaced a third bug nobody had connected: PokemonLookup's int8 fields use -1 for "absent", and int8(255) is -1. A clamped value entered the scan index as the sentinel for never encountered, silently changing which DNF filters matched. Every lookup field now saturates at a sentinel-safe ceiling instead of truncating — covering the whole ≥128 band, not just the exact ceiling — and Form gets its own helper because its -1 is the wildcard-form key rather than an absence marker. Atk/Def/Sta/Iv were reachable from rows already in the database, since a write-side clamp cannot heal stored data.

CI also now runs under the production build tag. It was running go test ./... without -tags go_json, so every golden-JSON proof in this PR was verified against stdlib while production serves through goccy. Nothing failed once corrected — goccy matches byte-for-byte across every fixture and adversarial case tested — but the proofs were unenforced against the shipping codec. A MariaDB service container now makes the round-trip tests actually execute rather than skip.

Rounds 5 and 6 — @Mygod flagged that the protobuf-to-plain-struct conversion drops unknown fields when an older binary rewrites golbat_internal. A guard was built to refuse the overwrite; @jfberry ruled it an overreach — the field is transitory, the flag defaults off, and losing fidelity there is harmless — so it was reverted. Four smaller asks landed instead: the proactive-IV skip now compares weather in one encoding rather than testing the lookup's -1 against the entity's 0; NullSeenType.UnmarshalJSON yields the inert Unknown sentinel rather than the destructive Unset; the warn throttles are reset by tests instead of swapped, closing a latent race; and three stale doc claims are corrected. CI also now vets the untagged build, which nothing else covered.

Testing

• Live MariaDB round-trip through the production pokemonBatchUpsertQuery: a row with every nullable column NULL, and a fully-populated row covering the full 64-bit cell_id, spawn_id, and float64 lat/lon precision.
• Wire compatibility for golbat_internal: a hand-written byte literal in the old grpc.PokemonInternal shape decodes through the new path, and the new write path produces bytes byte-identical to it.
• TestPokemonScanCoversEveryProtoField reflects over both structs and diffs exported field-name sets, so adding a proto field without mapping it fails loudly.
• Clamp convergence and clamp counting, each pinned separately.
• TestPokemonEntitySizes pins the sizes; TestPokemonUnderGCThreshold guards the 512 line.
• Green under both build tags, -race, golangci-lint, and the full suite.

statsCollector became an atomic.Pointer seeded with a noop at package init. A test needed to swap it, and decoder/init_test.go already documented that doing so races the stats aggregation worker, which reads the global on a ticker for the life of the process. This also retires a pre-existing race in station_battle_test.go.

Follow-ups

ProactiveIVSwitch's boosted-weather guard never fires — pre-existing on main, confirmed by @Mygod. boostedWeathers&uint8(1)<<w != 0 parses as (boostedWeathers & 1) << w, since Go gives & and << equal precedence. Every boostedWeatherLookup entry is even, so the guard is unconditionally false and newWeather is always 0 — the switch can only remove boosts, never apply them, and a currently-boosted pokemon whose new weather still boosts it gets wrongly de-boosted. Separately, NewWeather arrives as an unvalidated int32 from an open proto3 enum, and a negative shift count panics in a goroutine with no recover in its chain, so one malformed packet takes the process down. Both belong in their own PR, since the fix changes live behaviour immediately.

• The same negative-aliasing pattern is still live in fortRtree.go — bare int8/int16 casts on quest reward amounts, team and slots. The lookupInt8/lookupInt16 helpers added here apply verbatim.

• clampUint's derived ceiling would silently wrap for a future ~uint64 instantiation; a limit < 0 panic guard is cheap.

• The statsCollectorSet ordering guard doesn't cover the exported InitTypedQueues.

• TestUpdatePokemonLookupSaturatesClampedFields leaks a phantom entry into pokemonFormCount.

• The publish workflow's build-gate comment overclaims (branch pushes still publish per-branch images; the load-bearing case is fork-PR tokens), and tag-push releases now block on MariaDB service health.

• Bit-packing the numeric fields. Deliberately not done here. From 312 bytes, packing the small numerics into shared words reaches roughly 241 (the 256 class); adding a database row shim so Pvp and GolbatInternal can leave the struct reaches roughly 193 (the 208 class) — another ~112 bytes per pokemon. It has to be a build tag rather than a config flag, because Go fixes struct offsets at compile time and a runtime branch keeps the struct at the larger size, saving nothing. The cost is a build matrix of 4+ combinations and converting ~250 direct field reads to accessors in both layouts, so the readable build stops being readable. Worth its own PR against this measured baseline.

• Pvp and GolbatInternal were assessed and left alone. Two measured negative results, both now pinned in entity_sizes_test.go: Pvp is not dead between its write and its zeroing (the queue snapshot, the direct-write fallback and the webhook builder all read it), and narrowing it alone buys exactly zero allocated bytes, because 296 still lands in the 320 class and the next class down is 288.

• The intern table wants an alert threshold, not just a dashboard line. The length cap bounds entry size, not table count; a caller sending a million distinct well-formed usernames still grows it. golbat_intern_table_size is the instrument.

• A clamped value now lands exactly on PokemonLookup's missing-value sentinel — int8(255) = -1, and int16(65535) = -1 for Cp and Form. That aliases "clamped" with "unknown" in scan filters, which the pre-narrowing int8(300) = 44 did not. Narrow, latent, introduced here.

• Off-heap/arena storage for PokemonData deserves another look. It was ruled out earlier using ~5% for GC's share of CPU; at a measured 24.22% that arithmetic does not hold.

• Pokestop (1152 B) and Gym (968 B) are both far over the 512-byte threshold.

Design: docs/superpowers/specs/2026-08-16-pokemon-struct-packing-design.md

🤖 Generated with Claude Code

[jfberry]

Absolutely will need to be benchmarked; and against go 1.27 which changes allocation to be more efficient anyway.
But the memory saving alone may make this worthwhile

[jfberry]

Incidentally, since the null package already supports generics you may not need the newly defined nulltypes but rather just right-size in the current package

The embedded protobuf is absolutely ripe for change, it's a smell that it's in there in protobuf form - I did once replace it with a conventional structure and move to protobuf on demand (almost nowhere --> it is only marshalled when writing to disk, and the default is that it isn't).

The pokemonChanged value could be renamed to a generic name to encapsulate the change list, and rolled out to the other objects for a near free improvement.

[jfberry]

Finally, pull the SeenTypeCodeWild = 0 fix into this PR - and the re-widening of the webhook to float64 and reconversion seem like they are wasted steps for some misattributed compatibility requirement

[Mygod]

The protobuf-to-plain-struct conversion introduces forward-compatibility data loss when internal data is rewritten by an older binary. Otherwise, the targeted production and dbdebug decoder tests passed.

Review comment:

• [P2] Retain protobuf unknown fields during scan conversion — decoder/pokemon_scan.go:152-159
  When pokemon_internal_to_db is enabled during a rolling upgrade or rollback, rows may contain fields unknown to this binary. proto.Unmarshal previously retained those bytes in pokemon.internal, but rebuilding fresh PokemonInternal and PokemonScan messages here copies only known fields, so the next encounter overwrites golbat_internal without the newer data. Preserve the unknown bytes or message state when rebuilding.

Arguably doesn't need fixing?

[jfberry]

This (protobuf defence) change is an overreach. This is a transitory field, and there is no harm in losing fidelity when the default isn't even to store it.

[jfberry]

Round-6 verification is done and it's clean: both blockers are genuinely fixed and were verified empirically — the CI job is green with the MariaDB round-trip tests actually executing (not skipped) under both production tag sets, calculateIv now computes comparison, stores, and Iv from the same clamped values for every input, and the Unknown sentinel was traced through every non-test reader of SeenType and is inert at all of them, with COALESCE refusing overwrites at both write boundaries. All ten ranked items, the tidy-ups, and the title/body correction landed. Nothing in the new commits is merge-blocking — the undraft stands from our side. A few small items below, plus one thing that needs an author who predates this PR.

@Mygod — your advice wanted on ProactiveIVSwitch (pre-existing, not this PR's doing)

The review's differential pass sat on top of your boosted-weather guard and found something we'd like your read on before anyone fixes it, since you wrote this and know the intended behavior:

if boostedWeathers&uint8(1)<<weatherUpdate.NewWeather != 0 {
    newWeather = weatherUpdate.NewWeather
}

Go gives & and << equal precedence (left-assoc), so this parses as (boostedWeathers & 1) << NewWeather, not boostedWeathers & (1 << NewWeather). Every boostedWeatherLookup entry is even (bit 0 is weather NONE), so boostedWeathers & 1 is always 0 and the guard is unconditionally false — confirmed empirically, and it's on main, not introduced here. Net effect: newWeather is always 0, so the switch can only ever remove boosts, never apply them — and a currently-boosted pokemon whose new weather still boosts it gets wrongly de-boosted via repopulateIv(0, ...). The one-character fix (boostedWeathers&(uint8(1)<<weatherUpdate.NewWeather) != 0) changes live behavior immediately, which is why we'd rather have your confirmation of intent than just land it.

While you're looking: NewWeather arrives as an unvalidated int32 from an open proto3 enum, and a negative value used as a shift count panics — in a goroutine with no recover in its chain, so one malformed packet from any third-party scanner takes the whole process down. Clamping/validating to 0..7 before the shift (or making it uint8 at construction) closes both the crash and half the precedence question at once. Happy to take both in a follow-up PR if you agree with the intended semantics.

Small asks for this PR

1. The weather fast-path still compares two encodings of "absent". The cheap skip tests narrowUint8(newWeather) == pokemonLookup.Weather, where the lookup encodes NULL weather as -1; the entity-level guard after the lock uses int64OrZero(pokemon.Weather), which encodes NULL as 0. So for a pokemon with encounter values but NULL weather, a no-boost update (newWeather == 0) compares 0 == -1, fails the skip, takes the entity lock — and then the guard's 0 != 0 does nothing. One wasted entity lock per NULL-weather pokemon per weather flip in its cell, which is exactly the lookup-vs-entity disagreement the new comment says was closed. Fix: do the skip comparison in the lookup's own encoding (map "no weather" to -1 on the incoming side, or normalize the lookup value to 0 before comparing).

2. NullSeenType.UnmarshalJSON disagrees with Scan about the degraded state. Scan on an unknown string produces Unknown (inert everywhere — good), but UnmarshalJSON(null) produces code 0 = Unset, which is precisely the code this PR proved destructive (it licenses updateFromWild's rewrite and updateFromNearby's coordinate replacement). No production path unmarshals it today, but the type is exported and is the webhook payload's field type, so any replay tool or future path that round-trips a degraded record converts the safe sentinel into the destructive one. Make the two boundaries agree deliberately — one line. (The read-side behavior itself — degraded rows serving seen_type: null during a mixed-deployment window — is fine as-is; the column is COALESCE-protected.)

3. The warn-throttle globals — fix on the test side if possible. seenTypeScanWarns / seenTypeSetWarns are plain package-level pointers that production reads on row-load/decode paths while tests reassign them unsynchronised — the same shape init_test.go's own comment documents as a former data race for statsCollector. It's quiet today only because nothing loads rows concurrently with the swap. Preference: fix it without touching production code — e.g. have the tests inject their own DropReporter through a test-only seam rather than reassigning the global. If there's no clean way to do that, atomic.Pointer[util.DropReporter] is acceptable, but the production code shouldn't grow complexity to serve a test if it can be avoided.

4. Docs. Scan's comment overclaims the self-heal: wild and lure sightings deliberately don't match Unknown (correctly — matching it would reintroduce the downgrade), so the record heals on encounter or on reload only; reword it, because a reader making the documented behavior "real" by adding case SeenTypeCodeUnknown: to updateFromWild would rebuild exactly the bug the sentinel prevents. Also: webhooks.md's type table still lists seen_type as null.String (the one stale row in a table this PR otherwise corrected), and TestNullSeenTypeJSON's comment names huma as the consumer when the webhook sender's stdlib encoding/json is the real one.

Reviewed and accepted

The IV clamp-at-15 behavior (a hypothetical out-of-range reading becoming a stored 15) was considered and is fine: we don't receive fake IV data in practice, and golbat_field_clamped_total is sufficient signal if that ever changes. No action wanted.

Noted, non-blocking

CI no longer builds the untagged (!go_json) configuration anywhere — the one a bare go test ./... uses, including half of jsonenc; one extra untagged step (even go vet ./...) covers it. The same negative-aliasing pattern just fixed in the pokemon lookup is still live in fortRtree.go (bare int8/int16 casts on quest reward amounts / team / slots — the lookupInt8/16 fix applies verbatim). TestUpdatePokemonLookupSaturatesClampedFields leaks a phantom entry into pokemonFormCount (pair the cleanup with adjustPokemonFormCount(-1)). The statsCollectorSet guard doesn't cover exported InitTypedQueues (queues reading the collector at use time would delete the flag and the ordering requirement together). The publish workflow's build-gate comment overclaims (branch pushes still publish per-branch images; the load-bearing case is fork-PR tokens), and tag-push releases now block on MariaDB service health. clampUint's derived ceiling would silently wrap for a future ~uint64 instantiation (a limit < 0 panic guard is cheap). And the two throttle tests assert an exact count of 1 against a real 1-second wall-clock window — a rare-flake on loaded runners; assert >= 1 and < rows or inject the clock.

[Mygod]

Damn why is a bot tagging me and roasting my code. Yes, that should be fixed.

[jfberry]

On golbat_internal, settling this fully — including f0d4560a, not just the earlier defence: we don't need backward compatibility here in either direction. This is transitory operational state, off by default, and the contract we want is the simple one: load it and use it if it parses; if it doesn't, blank is fine; and when we write, we write this binary's view of the world unconditionally. The state evolves with the binary — last-writer-wins is the intended semantic, not an accident to defend against.

So please drop the refuse-to-overwrite guard as well (storedInternalHasUnknownFields and its skip path): 62dd77c5 was the right direction, and f0d4560a re-adds a defence we're deliberately choosing not to take. Beyond the ~100 lines, it costs an extra proto.Unmarshal of the stored bytes on every gated encounter save, and during the very window it protects, it prevents the running binary from persisting its own scan history on rows a newer build touched — which matters more to the running binary than preserving bytes it can't read. Plain rewrite, no guard.

[jfberry]

Correction to my previous comment: on verification, 62dd77c5 had already reverted f0d4560a completely — no refuse-to-overwrite guard exists at head (storedInternalHasUnknownFields, the reporter, the metric, and its tests are all gone, zero leftovers), and the net effect of the pair is just the rewriteGolbatInternal extraction, which verified behavior-identical to the previous inline block. So golbat_internal is already exactly where we want it — treat my previous comment as the policy record, not an ask. Apologies for the churn.

Verification of the rest of the post-round-6 commits — all four asks landed correctly: the weather skip and entity guard now agree on "unchanged" for every input (NULL-weather case included); UnmarshalJSON(null) produces the inert sentinel, re-traced through every decode switch and pinned by test; the throttle race is genuinely eliminated; and all three doc corrections check out against the code. The untagged go vet step runs and passes, and the range assertions landed.

One deviation to accept explicitly: the throttle fix value-types the two globals and adds a small Reset() to util.DropReporter — technically production surface, where the ask was test-side-only. But it removes the pointer indirection that existed solely for the test swap, so it meets the actual constraint (don't compromise main code — this simplifies it) and is safer than the atomic.Pointer fallback we'd sanctioned. Fine as landed.

Residual nits, take or leave, none blocking: the SetSeenType throttle test's upper bound (< 4) has much less headroom than its sibling's (< 500) against a badly-preempted runner straddling wall-clock seconds; TestNullSeenTypeUnmarshalNullIsInert marshals via stdlib encoding/json where the file's convention is jsonenc.Marshal; webhooks.md's seen_type prose says null is emitted "when unset" and should add the degraded-value-during-mixed-deployment case; and DropReporter.Reset's two stores aren't atomic as a unit — worth a doc line that callers must quiesce reporters first (only tests call it today).

That closes the review from our side.