refactor(server): extract the match telemetry recorder out of GameServer

[evanpelle]

Summary

Phase 3 of docs/GameServerRefactor.md, sixth and last module extraction (previous: #5114, #5116, #5117, #5122, #5124, #5126, #5142, #5143). A pure move with no behaviour change — the golden wire snapshot is untouched and MatchTelemetryIntegration.test.ts passes unmodified.

• src/server/MatchTelemetryRecorder.ts (new): one match's view of the telemetry stream. emit builds the envelope (schemaVersion, matchId, per-match sequence, observedAt, serverTick); a throwing emitter counts as a drop and still consumes a sequence number, so gaps mark drops. intentObserved / takeTickCounts keep the per-tick counters turn_committed reports; noteArchiveAttempted and matchFinished (once, however many times end() runs) cover the end of the match. identityFor, which keeps persistentID out of telemetry, is an exported function.
• GameServer: four fields and four private methods gone; every emit site passes turns.length explicitly where the old emitTelemetry default applied. Net −68 lines.
• tests/server/MatchTelemetryRecorder.test.ts (new, 7 tests): envelope/sequence/tick, the sequence gap a drop leaves, per-tick counters split by verdict, zero counts and clearing on take, finished-once with the archive flag, identityFor never carrying the persistentID.

With this, Phase 3 is complete: six modules in six PRs, golden snapshot unchanged throughout. GameServer.ts is 1,944 lines (from 2,365) and test reach-ins are 36 (from ~150), none of them spies on private methods; what remains is lifecycle and ingress state, which Phases 5–6 own.

Test plan

• npx vitest tests/server --run: 52 files / 522 tests green (was 51 / 516).
• Golden snapshot (GameServerWire.test.ts.snap): unchanged.
• Mutation check: removing the finished-once guard in matchFinished fails 1 test; restored.
• Full npm test: 31 failing files / 379 tests — identical to the pristine origin/main baseline (the pre-existing localStorage client failures); +6 passing.
• tsc --noEmit, prettier, oxlint, eslint clean.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5838ce5b-e1dc-49bf-88f9-36a6bd192ca5

📥 Commits

Reviewing files that changed from the base of the PR and between 6f82f99 and a31311a.

📒 Files selected for processing (4)

• docs/GameServerRefactor.md
• src/server/GameServer.ts
• src/server/MatchTelemetryRecorder.ts
• tests/server/MatchTelemetryRecorder.test.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

Walkthrough

GameServer telemetry handling now uses a dedicated MatchTelemetryRecorder. The recorder owns event envelopes, sequencing, intent counters, identity mapping, archive tracking, and one-time completion. Unit tests cover the new behavior, and the refactor documentation marks Phase 3 complete.

Changes

Telemetry recorder extraction

Layer / File(s)	Summary
Recorder contracts and state src/server/MatchTelemetryRecorder.ts, tests/server/MatchTelemetryRecorder.test.ts	Adds typed telemetry identity and tick-count handling. The recorder stamps events, tracks dropped emissions, counts intents, records archive attempts, and emits match_finished once.
GameServer telemetry integration src/server/GameServer.ts, docs/GameServerRefactor.md	Initializes the recorder and delegates all existing telemetry events and lifecycle updates to it. The refactor checklist records Phase 3 completion.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to a3131

This refactor moves telemetry state into a dedicated recorder while preserving sequencing, drop handling, identity minimization, and completion behavior; no actionable merge-blocking risk remains at the current head after normal checks and review.

Suggested reviewers: celant

Poem

Events find a focused home,
Ticks count softly as they roam,
Dropped signals leave a trace,
Finished matches close with grace,
Clean TypeScript keeps the pace.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: extracting the match telemetry recorder from GameServer.
Description check	✅ Passed	The description directly explains the MatchTelemetryRecorder extraction, the GameServer changes, added tests, and validation results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🤖 Claude Code Review

Verdict: No issues found — clean, faithful mechanical refactor. 0 findings (0 critical, 0 major, 0 minor).

This PR extracts GameServer's match-telemetry bookkeeping (event envelope/sequence stamping, per-tick intent counters, the finished-once guard, identityFor) into a new src/server/MatchTelemetryRecorder.ts class. I traced every call site against the pre-PR code on main:

• All emit sites that previously relied on emitTelemetry's default serverTick: number = this.turns.length parameter (match_opened, player_joined, match_started, match_finished) now pass this.turns.length explicitly at the same point in execution — values are unchanged.
• turn_committed and intent_observed sites already passed serverTick explicitly and are unchanged.
• matchFinished(totalTurns) reuses one argument for both payload.totalTurns and the envelope's serverTick; in the old code these were two separate reads of this.turns.length at the same call, so the emitted values are identical.
• The finished-once guard, the throw→"dropped" fallback (which still consumes a sequence number, preserving gap-as-drop semantics), the archive-attempted flag, and the per-tick counter get-or-zero/delete pair all carry over verbatim.
• identityFor still excludes persistentID from telemetry — no new leak.
• No dangling references to the removed GameServer fields/methods or the removed type imports; the new test file's helpers (makeClient, cid) and mock (RecordingEmitter) match the real APIs.

No CLAUDE.md violations: no src/core changes (test-required rule N/A), no user-visible strings (i18n rule N/A), and the new server unit test follows the same tests/server/ convention (GameServerHarness + hand-written mocks) used throughout that directory.

Four independent review passes (2× CLAUDE.md compliance, 2× bug-focused) each returned zero findings.