fix(statistics): stop suppressing victim deaths so the F13a cap stops undercounting

.planning/quick/260614-f14-f13a-deaths-fold-undercount/260614-f14-PLAN.md

@@ -0,0 +1,92 @@
+---
+quick_id: 260614-f14
+slug: f13a-deaths-fold-undercount
+status: complete
+date: 2026-06-14
+---
+
+# Quick Task 260614-f14: Fix the F13a deaths-fold undercount
+
+## Problem (root cause)
+
+After F12 + F13a + a full recalc, `sg` `deaths.total` runs ~5% **below** legacy,
+while games/kills are within ~1.5%. The decisive evidence is per-player: the
+recalc's `deaths.total` is 3-6% lower than the parser's own death-game signal
+(replays where the player's `player_counter.deaths_total > 0`, i.e. parser
+`player.d > 0`), which itself matches legacy. So the parser is right; the loss is
+in server-2's death **aggregation** (`calculatePlayerAndSquadAggregates`,
+`src/modules/statistics/service/service.ts`).
+
+The leak is the interaction between the F13a per-replay cap and the **pre-F13a
+`counterDeathReferences` suppression**, which F13a left in place:
+
+- `counterDeathReferences` (was `service.ts` ~88-95) is a `Set` of
+  `event.observedPlayerRef` for **every** `player_counter` whose
+  `counterDeaths(payload) !== undefined`. Note `counterDeaths` returns a DEFINED
+  `{total:0,…}` even for an explicit-zero counter (`deaths_total:0`); it is
+  `undefined` only when both `deaths_total` and `deaths_by_teamkills` are absent.
+- `applyVictimDeath` / `applySquadEvent` (victim branch) SUPPRESSED the victim
+  kill-row death when `counterDeathReferences.has(String(victim_entity_id))`.
+- `applyCounterEvent` resolves `playersByEntity.get(event.observedPlayerRef)` and
+  RETURNS EARLY (drops the death) when the player is unresolved.
+
+**The leak:** when an entity has a `player_counter` in `counterDeathReferences`
+(so the victim path is suppressed for that entity) but the counter path records
+nothing for the resolved victim — because the counter reports an explicit-zero
+`deaths_total` (`incrementDeathsByCounter` adds 0), or because `observedPlayerRef`
+does not resolve in `playersByEntity` — the real victim kill-row death is lost
+from **both** paths. The two paths also key differently (counter on
+`observedPlayerRef`, victim on `String(victim_entity_id)`), so any resolution
+mismatch between them leaks a death.
+
+Reproduced as a failing unit test before the fix: a replay with a
+`player_counter` `deaths_total:0` for entity 202 plus a `kill` row whose victim is
+202 (resolving to player-b) yields `deaths.total: 0`, where the target invariant
+requires `1`.
+
+## Target invariant
+
+Per canonical player, all-time `deaths.total` MUST equal the number of distinct
+replays in which that player died at least once (one-life: capped at 1/replay) —
+equivalently the parser's `d>0` replay count.
+
+## Approach
+
+With F13a's per-replay cap (`foldCappedDeaths` caps to <=1 `total` and <=1
+`by_teamkills` per replay) already in place, the `counterDeathReferences`
+suppression is **redundant and harmful**: double-counting between the counter path
+and the victim path now just collapses to 1 per replay anyway. The minimal fix is
+to **stop suppressing the victim path** and let both paths tally into the
+per-replay scratch maps; whichever path resolves the entity records the (capped)
+death.
+
+- Remove `counterDeathReferences` (construction, the `ReplayEventContext` field,
+  and both `.has(...)` suppression checks in `applyVictimDeath` and
+  `applySquadEvent`).
+- Both the counter path and the victim kill-row path now write into
+  `replayPlayerDeaths` / `replaySquadDeaths`; `foldCappedDeaths` collapses any
+  double-count to a single death and keeps `by_teamkills <= total`.
+
+This stays entirely in the **service** layer (the aggregation owner) — no parser,
+schema, repository SQL, or API-contract change
+(`solidstats-server-ts-conventions` §A: keep each layer doing its own job). F13a's
+cap is preserved: a player with `d=3` in one replay still counts 1.
+
+## Tasks
+
+1. **Tests first** (`service/tests/aggregates.test.ts`): add the failing leak repro
+   (zero counter + victim kill row → `total:1`), a teamkill variant asserting
+   `by_teamkills <= total`, and the N-distinct-replays invariant (died in 3
+   replays → `total:3`). Hoist a shared `counterAndKillReplay` fixture builder to
+   module scope.
+2. **Service** (`service/service.ts`): delete `counterDeathReferences` and the two
+   victim-path suppression checks; document the no-suppression rationale on the
+   per-replay scratch maps.
+
+## Acceptance
+
+- New leak/invariant tests pass; F13a cap tests still pass.
+- `pnpm lint`, `pnpm typecheck`, `pnpm test` (unit) green.
+- 100% coverage on the changed `service.ts` (unit-only run; full `test:coverage`
+  needs Docker for the integration files).
+- No OpenAPI / schema / parser-contract change.

.planning/quick/260614-f14-f13a-deaths-fold-undercount/260614-f14-SUMMARY.md

@@ -0,0 +1,90 @@
+---
+quick_id: 260614-f14
+slug: f13a-deaths-fold-undercount
+status: complete
+date: 2026-06-14
+---
+
+# Quick Task 260614-f14 — Summary
+
+## What changed
+
+Fixed the ~5% deaths undercount introduced by the F13a per-game cap (PR #17).
+Server-2 aggregation only — the Rust parser and its `d`/`td` artifact are
+untouched.
+
+- `src/modules/statistics/service/service.ts` —
+  `calculatePlayerAndSquadAggregates` no longer builds or consults
+  `counterDeathReferences`. The pre-F13a suppression of the victim kill-row death
+  path (`applyVictimDeath` and the `applySquadEvent` victim branch) is removed.
+  Both the counter path (`player_counter` → `deaths_total`/`by_teamkills`) and the
+  victim kill-row path now tally into the per-replay scratch maps
+  (`replayPlayerDeaths`, `replaySquadDeaths`); `foldCappedDeaths` still collapses
+  each replay's contribution to <=1 `total` and <=1 `by_teamkills`, so removing the
+  suppression cannot inflate counts — it only stops the leak.
+- `src/modules/statistics/service/tests/aggregates.test.ts` — added a hoisted
+  `counterAndKillReplay` fixture builder and three tests: the leak repro (zero
+  counter + victim kill row → `total:1`), a teamkill variant asserting
+  `by_teamkills <= total`, and the N-distinct-replays invariant (died in 3 replays
+  → `total:3`).
+
+## Root cause (confirmed)
+
+`counterDeathReferences` held `observedPlayerRef` for **every** `player_counter`
+with a DEFINED `counterDeaths(payload)` — including explicit-zero counters
+(`deaths_total:0` still yields `{total:0,…}`, which is defined). The victim path
+was suppressed whenever `counterDeathReferences.has(String(victim_entity_id))`,
+while the counter path (`applyCounterEvent`) dropped the death when its
+`observedPlayerRef` did not resolve in `playersByEntity` or contributed 0 for an
+explicit-zero counter. So for an entity with a present-but-non-contributing
+counter plus a real victim kill row, the death was lost from **both** paths. The
+two paths also key differently (counter: `observedPlayerRef`; victim:
+`String(victim_entity_id)`), so any resolution mismatch leaked a death. With
+F13a's per-replay cap already in place, the suppression was redundant — its only
+remaining effect was this leak.
+
+## The fix
+
+Stop suppressing the victim path; let whichever path resolves the entity record
+the death into the per-replay scratch map. The F13a cap (`foldCappedDeaths`)
+guarantees <=1 death and <=1 teamkill death per replay, so double-counting between
+the two paths collapses to exactly 1 and `by_teamkills` never exceeds `total`. The
+all-time `deaths.total` therefore equals the number of distinct replays the player
+died in — the target invariant and the legacy "games-died-in" semantics.
+
+## Conventions applied
+
+- **`solidstats-server-ts-conventions` §A** (layer responsibilities): the fix is
+  business logic in the **service** layer (the aggregation owner) — no leak into
+  the parser, repository SQL, schema, or API contract.
+- **`solidstats-server-ts-tests`** — service logic tested as **unit** (per-layer
+  map), using the file's in-line `AggregateReplayInput` fixtures, a hoisted shared
+  builder to satisfy `unicorn/consistent-function-scoping`, and no non-null
+  assertions.
+- **`solidstats-shared-testing-standards`** — strong oracles (exact `toEqual` on
+  the `deaths` object), one behavior per `it`, TDD (failing repro written first).
+
+## Validation
+
+- `pnpm lint` — green (full repo).
+- `pnpm typecheck` — green.
+- `pnpm format` — green.
+- `pnpm test` (unit) — 657 passed (78 files); `aggregates.test.ts` 14/14.
+- Coverage: `service.ts` is **100%** (statements/branches/functions/lines) from the
+  unit suite (scoped V8 run). The full CI gate `pnpm test:coverage` runs the
+  integration files (`*/tests/postgres.test.ts`, `src/test/integration/**`) which
+  require Docker/testcontainers; Docker is unavailable in this environment, so the
+  repo-wide 100% gate (which depends on those integration files for the
+  repository/route/infra layers) could not be exercised here. The changed file is
+  fully covered by unit tests.
+- No OpenAPI / schema / parser-contract change.
+
+## Acceptance
+
+- [x] Failing repro written first (zero counter + victim kill row → `total:1`).
+- [x] Victim-path suppression removed; both paths tally into the capped fold.
+- [x] One-life cap preserved (d=3 in one replay still counts 1).
+- [x] `by_teamkills <= total` (cap consistency) asserted and holds.
+- [x] N-distinct-replays invariant: died in 3 replays → `total:3`.
+- [x] `pnpm lint`, `pnpm typecheck`, `pnpm test` green; `service.ts` 100% coverage.
+- [x] No parser, schema, or API-contract change.

src/modules/statistics/service/service.ts

@@ -85,23 +85,22 @@ export function calculatePlayerAndSquadAggregates(
     const playersByEntity = new Map(
         replay.players.map((player) => [player.entityRef, player]),
       ),
-      counterDeathReferences = new Set(
-        replay.events.flatMap((event) =>
-          event.eventType === "player_counter" &&
-          counterDeaths(event.payload) !== undefined
-            ? [event.observedPlayerRef]
-            : [],
-        ),
-      ),
       // Per-replay death tallies. Solid Games are one-life: a player can die at
       // most once per game, so death evidence is summed within the replay here
       // and then folded into the cross-replay aggregate as a capped (<=1)
       // contribution. This keeps incap/revive artifacts and duplicate kill rows
       // from inflating a player's death count beyond games-died-in semantics.
+      //
+      // Both the counter path (player_counter -> deaths_total/by_teamkills) and
+      // the victim kill-row path tally into these maps without cross-path
+      // suppression: the per-replay cap in `foldCappedDeaths` collapses any
+      // double-count to a single death, so letting whichever path resolves the
+      // entity record the death avoids the F13a leak where a present-but-
+      // non-contributing counter (e.g. an explicit-zero deaths_total) suppressed
+      // a real victim death that the counter path then failed to credit.
       replayPlayerDeaths = new Map<string, DeathStats>(),
       replaySquadDeaths = new Map<string, DeathStats>(),
       eventContext = {
-        counterDeathReferences,
         playersByEntity,
         replayPlayerDeaths,
         replaySquadDeaths,
@@ -177,7 +176,6 @@ interface MutableSquadAggregate extends MutablePlayerAggregate {
 }
 
 interface ReplayEventContext {
-  counterDeathReferences: Set<string>;
   playersByEntity: Map<string, AggregatePlayerEvidence>;
   // Per-replay, uncapped death tallies keyed by playerId / squadId. Folded into
   // the cross-replay aggregate as a capped (<=1) contribution after the replay's
@@ -263,9 +261,6 @@ function applyVictimDeath(
   if (typeof victimEntityId !== "number") {
     return;
   }
-  if (context.counterDeathReferences.has(String(victimEntityId))) {
-    return;
-  }
   const victim = context.playersByEntity.get(String(victimEntityId));
   if (victim === undefined) {
     return;
@@ -304,9 +299,6 @@ function applySquadEvent(
   if (typeof victimEntityId !== "number") {
     return;
   }
-  if (context.counterDeathReferences.has(String(victimEntityId))) {
-    return;
-  }
   const victim = context.playersByEntity.get(String(victimEntityId));
   if (victim?.squadId !== undefined) {
     incrementDeaths(

src/modules/statistics/service/tests/aggregates.test.ts

@@ -30,6 +30,30 @@ function twoDeathEventsReplay(replayId: string): AggregateReplayInput {
   };
 }
 
+function counterAndKillReplay(replayId: string): AggregateReplayInput {
+  return {
+    events: [
+      {
+        eventType: "player_counter",
+        observedPlayerRef: "202",
+        payload: { deaths_total: 1 },
+        sourceRef: {},
+      },
+      {
+        eventType: "kill",
+        observedPlayerRef: "101",
+        payload: { victim_entity_id: 202 },
+        sourceRef: {},
+      },
+    ],
+    players: [
+      { entityRef: "101", playerId: "player-a" },
+      { entityRef: "202", playerId: "player-b" },
+    ],
+    replayId,
+  };
+}
+
 describe("calculatePlayerAndSquadAggregates", () => {
   it("calculates player and squad kills, teamkills, deaths, and replay counts", () => {
     const result = calculatePlayerAndSquadAggregates([
@@ -574,4 +598,101 @@ describe("calculatePlayerAndSquadAggregates", () => {
         .deaths,
     ).toEqual({ by_teamkills: 1, total: 1 });
   });
+
+  it("counts the death from the victim kill row when a present-but-non-contributing counter exists for the victim entity", () => {
+    // F13a leak repro. Entity 202 has a player_counter that reports an
+    // explicit-zero deaths_total (counterDeaths returns a DEFINED {total:0}, so
+    // the counter path adds nothing). A kill row proves 202 died and resolves to
+    // player-b. Under the buggy victim-path suppression the death was lost from
+    // BOTH paths and player-b ended with total:0 despite a real death event.
+    const result = calculatePlayerAndSquadAggregates([
+      {
+        events: [
+          {
+            eventType: "player_counter",
+            observedPlayerRef: "202",
+            payload: { deaths_total: 0 },
+            sourceRef: {},
+          },
+          {
+            eventType: "kill",
+            observedPlayerRef: "101",
+            payload: { victim_entity_id: 202 },
+            sourceRef: {},
+          },
+        ],
+        players: [
+          { entityRef: "101", playerId: "player-a" },
+          { entityRef: "202", playerId: "player-b", squadId: "squad-b" },
+        ],
+        replayId: "replay-1",
+      },
+    ]);
+
+    expect(
+      result.playerStats.find((player) => player.playerId === "player-b")?.stats
+        .deaths,
+    ).toEqual({ by_teamkills: 0, total: 1 });
+    expect(
+      result.squadStats.find((squad) => squad.squadId === "squad-b")?.stats
+        .deaths,
+    ).toEqual({ by_teamkills: 0, total: 1 });
+  });
+
+  it("counts the teamkill death from the victim kill row when a zero counter exists for the victim entity (byTeamkills <= total)", () => {
+    // Same leak as above, but the kill row is a teamkill: byTeamkills must equal
+    // total (both 1), never exceed it. The zero counter must not suppress it.
+    const result = calculatePlayerAndSquadAggregates([
+      {
+        events: [
+          {
+            eventType: "player_counter",
+            observedPlayerRef: "202",
+            payload: { deaths_by_teamkills: 0, deaths_total: 0 },
+            sourceRef: {},
+          },
+          {
+            eventType: "teamkill",
+            observedPlayerRef: "101",
+            payload: { victim_entity_id: 202 },
+            sourceRef: {},
+          },
+        ],
+        players: [
+          { entityRef: "101", playerId: "player-a" },
+          { entityRef: "202", playerId: "player-b", squadId: "squad-b" },
+        ],
+        replayId: "replay-1",
+      },
+    ]);
+
+    const playerDeaths = result.playerStats.find(
+      (player) => player.playerId === "player-b",
+    )?.stats.deaths;
+    expect(playerDeaths).toEqual({ by_teamkills: 1, total: 1 });
+    expect(playerDeaths?.by_teamkills ?? 0).toBeLessThanOrEqual(
+      playerDeaths?.total ?? 0,
+    );
+    expect(
+      result.squadStats.find((squad) => squad.squadId === "squad-b")?.stats
+        .deaths,
+    ).toEqual({ by_teamkills: 1, total: 1 });
+  });
+
+  it("counts deaths.total equal to the number of distinct replays a player died in (no loss under the counter+victim paths)", () => {
+    // The invariant: deaths.total === count of replays where the player died at
+    // least once. Each replay below has a resolving counter AND a victim kill row
+    // for the same entity; the player must be credited exactly one death per
+    // replay, three across three replays.
+    const result = calculatePlayerAndSquadAggregates([
+      counterAndKillReplay("replay-1"),
+      counterAndKillReplay("replay-2"),
+      counterAndKillReplay("replay-3"),
+    ]);
+
+    expect(
+      result.playerStats.find((player) => player.playerId === "player-b")?.stats
+        .deaths,
+    ).toEqual({ by_teamkills: 0, total: 3 });
+  });
 });