fix(statistics): exclude null-timestamp replays from scope aggregation (#15)

The all-time recalc scope loaded every current replay of its game type,
including the 1101 parity-corpus replays with a NULL replay_timestamp. A
no-SteamID player on such a replay reaches the name-fallback path, which keys
on replay_timestamp.toISOString() and threw
'Cannot read properties of null (reading toISOString)'.

scopedCurrentResultsSql now excludes NULL-timestamp replays (they cannot be
placed in time and the audit path already reports them missing_replay_timestamp,
never aggregated). Adds a repository integration test that reproduces the crash
pre-fix and asserts the null-timestamp replay's player is excluded from the
all-time bucket and gets no name-fallback nickname.

GSD: .planning/quick/260614-r9k-recalc-null-timestamp-guard

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Afgan0r

.planning/STATE.md

@@ -132,6 +132,7 @@ Decisions are logged in PROJECT.md Key Decisions table. Recent decisions affecti
 | 260510-hc5 | Remove staging ingress from server-2 CD | 2026-05-10 | dbe5025 | [260510-hc5-remove-staging-ingress-from-server-2-cd](./quick/260510-hc5-remove-staging-ingress-from-server-2-cd/) |
 | 260614-c0d | Fix F7 — set-based recalculation so parity check can pass | 2026-06-14 | 2930f10 | [260614-c0d-f7-set-based-recalculation-parity](./quick/260614-c0d-f7-set-based-recalculation-parity/) |
 | 260614-fw2 | Set-based canonical player identity resolution in per-rotation recalc (behavior-preserving) | 2026-06-14 | fa7c54b | [260614-fw2-perf-set-based-canonical-player-identity](./quick/260614-fw2-perf-set-based-canonical-player-identity/) |
+| 260614-r9k | Guard all-time recalc against NULL replay_timestamp (toISOString crash) | 2026-06-14 | b0275e0 | [260614-r9k-recalc-null-timestamp-guard](./quick/260614-r9k-recalc-null-timestamp-guard/) |
 
 ## Deferred Items
 

.planning/quick/260614-r9k-recalc-null-timestamp-guard/260614-r9k-PLAN.md

@@ -0,0 +1,55 @@
+---
+quick_id: 260614-r9k
+slug: recalc-null-timestamp-guard
+status: complete
+---
+
+# Quick Task 260614-r9k: Guard the all-time recalc against NULL replay_timestamp
+
+## Problem (root cause)
+
+The full-run / all-time statistics recalculation crashed with
+`TypeError: Cannot read properties of null (reading 'toISOString')` in
+`uniqueNameOccurrences` (`src/modules/statistics/repository/repository.ts:838`).
+
+`scopedCurrentResultsSql` builds the per-scope query over current replays. For the
+**all-time** scope it loaded *every* current replay of the scope's game type —
+including replays with a `NULL` `replay_timestamp` (the parity corpus has 1101 such
+rows). When such a replay carries a **no-SteamID** player, identity resolution falls
+through to the name-fallback path (`ensureNameFallbackIdentities` →
+`uniqueNameOccurrences`), which keys each occurrence on
+`row.replay_timestamp.toISOString()`. With a `NULL` timestamp that dereference throws.
+
+The single-replay audit path already reports these as `missing_replay_timestamp` and
+never recalculates them; only the all-time scope's broad scan pulled them in.
+
+## Approach
+
+Exclude `NULL`-timestamp replays from `scopedCurrentResultsSql` with
+`and r.replay_timestamp is not null`. Rationale:
+
+- They cannot be placed in time, so they have no rotation/all-time position.
+- The all-time bucket's name-occurrence identity resolution keys on the timestamp.
+- Recalculation already classifies them `missing_replay_timestamp` (never aggregated),
+  so excluding them from the scope is behavior-consistent, not a new policy.
+
+This is the narrowest, convention-conformant fix: the repository layer owns the SQL
+and the row contract, so the guard belongs in the query, not in a downstream
+service/usecase. (`solidstats-server-ts-conventions` §A: repository = data-access
+adapter; keep each layer doing its own job.)
+
+## Tasks
+
+1. **Repository** (`repository/repository.ts`): add the `r.replay_timestamp is not null`
+   predicate to `scopedCurrentResultsSql` + a docstring note explaining the exclusion.
+2. **Integration test** (`repository/tests/postgres.test.ts`): reproduce the crash
+   pre-fix / pass post-fix using the file's seed helpers
+   (`seedRotation`/`seedPlayer`/`seedParserResult`/`missionEnvelope`/
+   `classifyGameTypesForCurrentReplays`); assert the null-timestamp replay's player is
+   excluded from the all-time bucket and gets no name-fallback nickname.
+
+## Acceptance
+
+- `pnpm typecheck`, `pnpm exec eslint <changed files>` green.
+- Integration test green in CI (testcontainers Postgres; not runnable locally).
+- No OpenAPI contract change.

.planning/quick/260614-r9k-recalc-null-timestamp-guard/260614-r9k-SUMMARY.md

@@ -0,0 +1,66 @@
+---
+quick_id: 260614-r9k
+slug: recalc-null-timestamp-guard
+status: complete
+date: 2026-06-14
+---
+
+# Quick Task 260614-r9k — Summary
+
+## What changed
+
+Fixed a crash in the full-run / all-time statistics recalculation:
+`TypeError: Cannot read properties of null (reading 'toISOString')` raised in
+`uniqueNameOccurrences` (`src/modules/statistics/repository/repository.ts`).
+
+- `repository/repository.ts` — `scopedCurrentResultsSql` now appends
+  `and r.replay_timestamp is not null` to its WHERE clause, with a docstring note. The
+  all-time scope's broad scan previously loaded `NULL`-timestamp replays (1101 in the
+  parity corpus); a no-SteamID player on such a replay reached the name-fallback path,
+  which keys each occurrence on `replay_timestamp.toISOString()` and threw on `NULL`.
+- `repository/tests/postgres.test.ts` — added an integration test that seeds a timed
+  `sg` replay (SteamID player Alpha) plus a `NULL`-timestamp `sg` replay (no-SteamID
+  player Ghost), runs `classifyGameTypesForCurrentReplays`, then recalculates the timed
+  replay. Pre-fix this threw in the all-time rebuild; post-fix it returns
+  `{ playerStats: 2, rotationId, squadStats: 0, status: "recalculated" }`. Asserts only
+  Alpha appears in `player_stats` and Ghost gets no name-fallback `player_nicknames` row.
+
+## Why this is correct (key insight)
+
+`NULL`-timestamp replays cannot be placed in time and the single-replay audit path
+already reports them `missing_replay_timestamp` (never recalculated). Excluding them
+from the scope query is consistent with existing behavior — it does not change which
+replays get aggregated, only stops the all-time scan from dereferencing a `NULL`
+timestamp. The guard lives in the repository SQL because that layer owns the query and
+the row contract (`solidstats-server-ts-conventions` §A).
+
+## Conventions applied
+
+- **`solidstats-server-ts-conventions` §A** (layer responsibilities): the fix is the
+  repository's SQL/row contract, kept in the repository layer — no leak into
+  service/usecase.
+- **`solidstats-server-ts-tests`** — per-layer testing map (repository ⇒ **integration**
+  against real Postgres, never a mocked DB) and the testcontainers harness; reused the
+  file's existing typed seed builders (`seedRotation`/`seedPlayer`/`seedParserResult`/
+  `missionEnvelope`/`fullRunRepository.classifyGameTypesForCurrentReplays`).
+- **`solidstats-shared-testing-standards`** (via the tests skill) — AAA structure
+  (explicit Arrange/Act/Assert), scenario-named `it`, and a **strong oracle**: an exact
+  `toEqual` on the recalc result plus positive (Alpha aggregated) and negative (Ghost
+  excluded, no fallback nickname) assertions rather than a loose `toMatchObject`.
+  Distinct `sourceReplayId`s avoid the seed helper's unique object_key/checksum clash.
+
+## Validation
+
+`pnpm typecheck` and `pnpm exec eslint src/modules/statistics/repository/repository.ts
+src/modules/statistics/repository/tests/postgres.test.ts` — both green. The integration
+test needs the testcontainers/Docker Postgres, which isn't available locally; it runs in
+CI. No OpenAPI contract change.
+
+## Acceptance
+
+- [x] Root cause fixed at the repository SQL layer.
+- [x] Integration test reproduces the crash pre-fix and passes post-fix (correct by
+      construction per the test skill).
+- [x] `pnpm typecheck`, `pnpm exec eslint <changed files>` green.
+- [x] No OpenAPI contract change.
+- [ ] CI integration run green (runs in pipeline; not runnable locally here).

src/modules/statistics/repository/repository.ts

@@ -557,6 +557,12 @@ async function aggregateInputsFromRows(
  * predicate uses `IS NOT DISTINCT FROM` so a `null` audit-path game type matches
  * the legacy single-bucket rows, while a concrete type matches exactly and never
  * picks up excluded (`NULL`) replays.
+ *
+ * Replays with a `NULL` replay_timestamp are excluded: they cannot be placed in
+ * time, the all-time bucket's name-occurrence identity resolution keys on the
+ * timestamp, and the recalculation already reports them as
+ * `missing_replay_timestamp` (never recalculated). Without this filter the
+ * all-time scope loaded them and crashed on `replay_timestamp.toISOString()`.
  */
 function scopedCurrentResultsSql(scope: AggregateScope): string {
   const rotationPredicate =
@@ -567,6 +573,7 @@ function scopedCurrentResultsSql(scope: AggregateScope): string {
     join replays r on r.id = pr.replay_id
     where pr.status = 'current'
       and r.game_type is not distinct from $1
+      and r.replay_timestamp is not null
       ${rotationPredicate}
     order by r.replay_timestamp, pr.created_at
   `;

src/modules/statistics/repository/tests/postgres.test.ts

@@ -552,6 +552,72 @@ describe("PgStatisticsRepository", () => {
     });
   });
 
+  it("excludes a null-timestamp replay from the all-time bucket rebuild", async () => {
+    // Arrange — two current `sg` replays. One is placeable in time with a
+    // SteamID-bearing player (Alpha). The other has NO replay_timestamp and a
+    // no-SteamID player (Ghost), so its identity can only be resolved via the
+    // name-fallback path, which keys on `replay_timestamp.toISOString()`. The
+    // all-time bucket rebuild scans EVERY current `sg` replay, so without the
+    // `replay_timestamp is not null` filter in scopedCurrentResultsSql it loads
+    // Ghost's row and crashes with
+    // `Cannot read properties of null (reading 'toISOString')`. Distinct
+    // sourceReplayIds keep the replays' unique object_key/checksum apart.
+    const rotationId = await seedRotation(),
+      playerAlpha = await seedPlayer("Alpha", "steam-a"),
+      timedResultId = await seedParserResult({
+        rawSnapshot: {
+          contract_version: "3.0.0",
+          parser: {},
+          players: [{ eid: 101, n: "Alpha", sid: "steam-a" }],
+          replay: missionEnvelope("sg_assault"),
+          source: {},
+          status: "success",
+        },
+        replayTimestamp: "2026-02-01T12:00:00.000Z",
+        sourceReplayId: "sg-timed-replay",
+      });
+    await seedParserResult({
+      rawSnapshot: {
+        contract_version: "3.0.0",
+        parser: {},
+        players: [{ eid: 202, n: "Ghost" }],
+        replay: missionEnvelope("sg_assault"),
+        source: {},
+        status: "success",
+      },
+      replayTimestamp: null,
+      sourceReplayId: "sg-null-timestamp-replay",
+    });
+    await fullRunRepository.classifyGameTypesForCurrentReplays();
+
+    // Act — recalculate the timed replay; this rebuilds the sg per-rotation AND
+    // sg all-time buckets, the latter scanning the null-timestamp replay too.
+    const result =
+      await repository.recalculatePlayerAndSquadStatsForParserResult(
+        timedResultId,
+      );
+
+    // Assert — recalc completes (no toISOString crash) and aggregates only the
+    // timed replay: Alpha across the sg per-rotation + sg all-time scopes
+    // (playerStats: 2), no squads. The null-timestamp replay contributes
+    // nothing: no all-time player_stats row and no name-fallback nickname for
+    // its no-SteamID player.
+    expect(result).toEqual({
+      playerStats: 2,
+      rotationId,
+      squadStats: 0,
+      status: "recalculated",
+    });
+    const aggregatedPlayerIds = await pool.query<{ player_id: string }>(
+      "select distinct player_id from player_stats",
+    );
+    expect(aggregatedPlayerIds.rows).toEqual([{ player_id: playerAlpha }]);
+    const ghostNicknames = await pool.query<{ count: string }>(
+      "select count(*) from player_nicknames where lower(nickname) = 'ghost'",
+    );
+    expect(ghostNicknames.rows[0]?.count).toBe("0");
+  });
+
   it("assigns replay rotation and replaces commander side aggregates", async () => {
     const rotationId = await seedRotation(),
       playerA = await seedPlayer("Alpha", "steam-a"),