diff --git a/ai/tasks/done/TASK-247-implement-real-player-active-time-and-kpm.md b/ai/tasks/done/TASK-247-implement-real-player-active-time-and-kpm.md new file mode 100644 index 0000000..6b02ba7 --- /dev/null +++ b/ai/tasks/done/TASK-247-implement-real-player-active-time-and-kpm.md @@ -0,0 +1,142 @@ +# TASK-247 - Implement real player active time and historical KPM + +## Summary + +This task implements the first real KPM base for historical match detail using reconstructed connection intervals from the RCON AdminLog materialization flow. + +It does not use total match duration as a substitute for player time. + +## Files Read First + +- `backend/app/rcon_admin_log_materialization.py` +- `backend/app/rcon_admin_log_storage.py` +- `backend/app/rcon_historical_read_model.py` +- `backend/app/payloads.py` +- `backend/app/postgres_rcon_storage.py` +- `backend/app/config.py` +- `backend/tests/test_rcon_materialization_pipeline.py` +- `backend/tests/test_current_match_payload.py` +- `backend/tests/test_rcon_admin_log_storage.py` +- `frontend/assets/js/historico-partida.js` +- `frontend/historico-partida.html` +- `docs/REAL_KPM_PLAYER_ACTIVE_SECONDS_DESIGN.md` +- `docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md` + +## Problem + +Historical match detail could not show a real player KPM because the backend had no player-level active time persisted in the RCON materialized match facts. + +Using total match duration would have reintroduced a false KPM. + +## Decision + +Use the existing RCON AdminLog materialization path as the source of truth. + +For each player inside one materialized match: + +- keep `first_seen_server_time` +- keep `last_seen_server_time` +- reconstruct connected intervals inside the match window +- persist `player_active_seconds` from those intervals when they are reliable +- mark `active_time_source` according to the interval quality + +Reliable interval sources: + +- `connection_intervals` +- `connection_intervals_carryover` + +Fallback only, not valid for real KPM: + +- `event_span_fallback` +- `unavailable` + +Then expose KPM in the historical match detail payload only when active time is present and reaches a minimum threshold. + +## Persistence + +Added to `rcon_match_player_stats`: + +- `player_active_seconds` +- `active_time_source` + +This is applied for both SQLite and PostgreSQL schema initialization/migration. + +Legacy rows remain compatible. They keep `player_active_seconds = null` until they are rematerialized or replaced by newer matches. + +## KPM Rules + +- Formula: + - `kills / (player_active_seconds / 60)` +- Minimum threshold: + - `HLL_KPM_MIN_ACTIVE_SECONDS` + - default `60` +- If `player_active_seconds` is missing: + - `kpm = null` + - `kpm_status = missing_active_time` +- If active time comes only from fallback event span: + - `kpm = null` + - `kpm_status = missing_connection_intervals` +- If `player_active_seconds < 60`: + - `kpm = null` + - `kpm_status = insufficient_active_time` +- If active time is valid: + - `kpm` is rounded to 2 decimals + - `kpm_status = ready` + - only when `active_time_source` is `connection_intervals` or `connection_intervals_carryover` + +## Interval Rules + +- connect inside match: + - open interval at `connected.server_time` +- disconnect inside match: + - close interval at `disconnected.server_time` +- connected before match start and no pre-start disconnect after that: + - carry interval from `match_start` +- still connected at match end: + - close interval at `match_end` +- reconnections: + - sum all intervals +- inconsistent or incomplete connection evidence: + - do not mark KPM as ready + - keep `event_span_fallback` only as auxiliary observed time + +## Squad Time + +Not implemented. + +This task reviewed the possibility of discounting time without squad/unit/role, but the current historical source is not reliable enough to compute `squad_active_seconds` safely. That remains future work. + +## Frontend + +`historico-partida.js` now shows a `KPM` stat chip in the expanded player panel only when `kpm_status == "ready"`. + +Old matches without active time do not show a fake `0.00`. + +## Validation + +Executed: + +- `python -m compileall backend/app` +- `cd backend; python -m unittest tests.test_current_match_payload` +- `cd backend; python -m unittest tests.test_rcon_admin_log_storage` +- `cd backend; python -m unittest tests.test_historical_snapshot_refresh` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_materialization_migrates_existing_player_stats_schema_with_active_time_columns` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_match_detail_keeps_kpm_missing_for_legacy_rows_without_active_time` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_match_detail_read_model_hides_raw_player_ids` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_active_time_counts_full_match_for_player_connected_before_start` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_active_time_counts_until_disconnect_for_player_connected_before_start` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_active_time_counts_from_connect_until_match_end` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_active_time_sums_multiple_connection_intervals` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_active_time_uses_event_span_fallback_without_ready_kpm_when_connection_intervals_missing` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_kpm_is_null_when_active_time_is_below_threshold` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_kpm_payload_helper_returns_ready_for_ten_kills_in_ten_minutes` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_kpm_payload_helper_returns_zero_for_zero_kills_with_valid_active_time` +- `cd backend; python -m unittest tests.test_rcon_materialization_pipeline.RconMaterializationPipelineTests.test_kpm_payload_helper_returns_missing_when_active_time_is_null` +- `node --check frontend/assets/js/historico-partida.js` + +## Notes + +- No scheduler changes were required. +- No RCON host/port/server config changes were made. +- No weapon/icon assets were touched. +- Historical KPM remains forward-valid only. Old rows stay null until real interval evidence exists. diff --git a/backend/app/config.py b/backend/app/config.py index 51ef311..e234b7a 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -47,6 +47,7 @@ DEFAULT_RCON_HISTORICAL_CAPTURE_MAX_RETRIES = 2 DEFAULT_RCON_HISTORICAL_CAPTURE_RETRY_DELAY_SECONDS = 15 DEFAULT_RCON_CURRENT_MATCH_CAPTURE_INTERVAL_SECONDS = 5 DEFAULT_RCON_CURRENT_MATCH_WRITER_LOCK_TIMEOUT_SECONDS = 4.0 +DEFAULT_KPM_MIN_ACTIVE_SECONDS = 60 DEFAULT_RCON_BACKFILL_CHUNK_HOURS = 6 DEFAULT_RCON_BACKFILL_SLEEP_SECONDS = 1.0 DEFAULT_RCON_BACKFILL_MAX_DAYS_BACK = 45 @@ -677,6 +678,15 @@ def get_rcon_current_match_writer_lock_timeout_seconds() -> float: ) +def get_kpm_min_active_seconds() -> int: + """Return the minimum observed active seconds required before KPM is considered valid.""" + return _read_int_env( + "HLL_KPM_MIN_ACTIVE_SECONDS", + str(DEFAULT_KPM_MIN_ACTIVE_SECONDS), + minimum=1, + ) + + def get_rcon_backfill_chunk_hours() -> int: """Return the AdminLog backfill chunk size in hours.""" return _read_int_env( diff --git a/backend/app/postgres_rcon_storage.py b/backend/app/postgres_rcon_storage.py index 1b2a2cf..bbea491 100644 --- a/backend/app/postgres_rcon_storage.py +++ b/backend/app/postgres_rcon_storage.py @@ -183,6 +183,8 @@ CREATE TABLE IF NOT EXISTS rcon_match_player_stats ( death_by_json TEXT NOT NULL DEFAULT '{}', first_seen_server_time BIGINT, last_seen_server_time BIGINT, + player_active_seconds INTEGER, + active_time_source TEXT, created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP, UNIQUE(target_key, match_key, player_id) @@ -415,6 +417,14 @@ BEGIN END $$; """ +POSTGRES_RCON_MATCH_PLAYER_STATS_ACTIVE_TIME_MIGRATION_SQL = """ +ALTER TABLE rcon_match_player_stats +ADD COLUMN IF NOT EXISTS player_active_seconds INTEGER; + +ALTER TABLE rcon_match_player_stats +ADD COLUMN IF NOT EXISTS active_time_source TEXT; +""" + def initialize_postgres_rcon_storage() -> None: """Create deterministic PostgreSQL schema for migrated RCON domains.""" @@ -422,6 +432,7 @@ def initialize_postgres_rcon_storage() -> None: with connection.cursor() as cursor: cursor.execute(RCON_SCHEMA_SQL) cursor.execute(POSTGRES_ANNUAL_RANKING_SCHEMA_MIGRATION_SQL) + cursor.execute(POSTGRES_RCON_MATCH_PLAYER_STATS_ACTIVE_TIME_MIGRATION_SQL) @contextmanager diff --git a/backend/app/rcon_admin_log_materialization.py b/backend/app/rcon_admin_log_materialization.py index 54634e8..f04f3d6 100644 --- a/backend/app/rcon_admin_log_materialization.py +++ b/backend/app/rcon_admin_log_materialization.py @@ -96,6 +96,8 @@ def initialize_rcon_materialized_storage(*, db_path: Path | None = None) -> Path death_by_json TEXT NOT NULL DEFAULT '{}', first_seen_server_time INTEGER, last_seen_server_time INTEGER, + player_active_seconds INTEGER, + active_time_source TEXT, created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, UNIQUE(target_key, match_key, player_id) @@ -153,6 +155,7 @@ def initialize_rcon_materialized_storage(*, db_path: Path | None = None) -> Path ON rcon_annual_ranking_snapshot_items(snapshot_id, player_id); """ ) + _ensure_materialized_player_stat_columns(connection) return resolved_path @@ -752,6 +755,12 @@ def _derive_player_stats_for_match( stats = [] for player in players.values(): + active_time = _build_player_active_time_payload( + connection, + match=match, + player=player, + match_rows=rows, + ) stats.append( { "target_key": match["target_key"], @@ -769,6 +778,8 @@ def _derive_player_stats_for_match( "death_by_json": _dump_counter(player["death_by"]), "first_seen_server_time": player.get("first_seen_server_time"), "last_seen_server_time": player.get("last_seen_server_time"), + "player_active_seconds": active_time["player_active_seconds"], + "active_time_source": active_time["active_time_source"], } ) return stats @@ -781,8 +792,9 @@ def _insert_player_stat(connection: sqlite3.Connection, stat: dict[str, object]) target_key, match_key, player_id, player_name, team, kills, deaths, teamkills, deaths_by_teamkill, weapons_json, death_by_weapons_json, most_killed_json, death_by_json, - first_seen_server_time, last_seen_server_time - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + first_seen_server_time, last_seen_server_time, + player_active_seconds, active_time_source + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( stat["target_key"], @@ -800,6 +812,8 @@ def _insert_player_stat(connection: sqlite3.Connection, stat: dict[str, object]) stat["death_by_json"], stat.get("first_seen_server_time"), stat.get("last_seen_server_time"), + stat.get("player_active_seconds"), + stat.get("active_time_source"), ), ) @@ -846,6 +860,171 @@ def _touch_player(player: dict[str, object], server_time: int | None) -> None: player["last_seen_server_time"] = server_time if last_seen is None else max(last_seen, server_time) +def _calculate_event_span_seconds( + *, + first_seen_server_time: object, + last_seen_server_time: object, +) -> int | None: + first_seen = _coerce_int(first_seen_server_time) + last_seen = _coerce_int(last_seen_server_time) + if first_seen is None or last_seen is None: + return None + return max(0, last_seen - first_seen) + + +def _build_player_active_time_payload( + connection: sqlite3.Connection, + *, + match: dict[str, object], + player: dict[str, object], + match_rows: list[sqlite3.Row], +) -> dict[str, object]: + lower = _coerce_int(match.get("started_server_time")) + upper = _coerce_int(match.get("ended_server_time")) + fallback_seconds = _calculate_event_span_seconds( + first_seen_server_time=player.get("first_seen_server_time"), + last_seen_server_time=player.get("last_seen_server_time"), + ) + player_id = str(player.get("player_id") or "").strip() + + if lower is None or upper is None or upper < lower: + return { + "player_active_seconds": fallback_seconds, + "active_time_source": "event_span_fallback" if fallback_seconds is not None else "unavailable", + } + + if not player_id or player_id.startswith("name:"): + return { + "player_active_seconds": fallback_seconds, + "active_time_source": "event_span_fallback" if fallback_seconds is not None else "unavailable", + } + + interval_events = _collect_player_connection_events_from_match_rows( + match_rows, + player_id=player_id, + ) + prior_connected = _player_was_connected_at_match_start( + connection, + target_key=str(match["target_key"]), + player_id=player_id, + match_start_server_time=lower, + ) + interval_seconds, interval_source = _calculate_connection_interval_active_seconds( + match_start_server_time=lower, + match_end_server_time=upper, + prior_connected=prior_connected, + interval_events=interval_events, + ) + if interval_source is not None: + return { + "player_active_seconds": interval_seconds, + "active_time_source": interval_source, + } + + return { + "player_active_seconds": fallback_seconds, + "active_time_source": "event_span_fallback" if fallback_seconds is not None else "unavailable", + } + + +def _collect_player_connection_events_from_match_rows( + rows: list[sqlite3.Row], + *, + player_id: str, +) -> list[tuple[str, int]]: + events: list[tuple[str, int]] = [] + for row in rows: + event_type = str(row["event_type"] or "") + if event_type not in {"connected", "disconnected"}: + continue + payload = _json_object(row["parsed_payload_json"]) + event_player_id = str(payload.get("player_id") or "").strip() + server_time = _coerce_int(row["server_time"]) + if event_player_id == player_id and server_time is not None: + events.append((event_type, server_time)) + events.sort(key=lambda item: item[1]) + return events + + +def _player_was_connected_at_match_start( + connection: sqlite3.Connection, + *, + target_key: str, + player_id: str, + match_start_server_time: int, +) -> bool: + row = connection.execute( + """ + SELECT event_type + FROM rcon_admin_log_events + WHERE target_key = ? + AND server_time IS NOT NULL + AND server_time < ? + AND event_type IN ('connected', 'disconnected') + AND parsed_payload_json LIKE ? + ORDER BY server_time DESC, id DESC + LIMIT 1 + """, + ( + target_key, + match_start_server_time, + f'%"player_id":"{player_id}"%', + ), + ).fetchone() + return bool(row and row["event_type"] == "connected") + + +def _calculate_connection_interval_active_seconds( + *, + match_start_server_time: int, + match_end_server_time: int, + prior_connected: bool, + interval_events: list[tuple[str, int]], +) -> tuple[int | None, str | None]: + open_since = match_start_server_time if prior_connected else None + total_seconds = 0 + used_carryover = prior_connected + has_reliable_intervals = prior_connected + + for event_type, server_time in interval_events: + clamped_time = max(match_start_server_time, min(match_end_server_time, server_time)) + if event_type == "connected": + if open_since is None: + open_since = clamped_time + has_reliable_intervals = True + continue + if open_since is None: + continue + total_seconds += max(0, clamped_time - open_since) + open_since = None + has_reliable_intervals = True + + if open_since is not None: + total_seconds += max(0, match_end_server_time - open_since) + + if not has_reliable_intervals: + return None, None + return ( + total_seconds, + "connection_intervals_carryover" if used_carryover else "connection_intervals", + ) + + +def _ensure_materialized_player_stat_columns(connection: sqlite3.Connection) -> None: + columns = { + row["name"] + for row in connection.execute("PRAGMA table_info(rcon_match_player_stats)").fetchall() + } + if "player_active_seconds" not in columns: + connection.execute( + "ALTER TABLE rcon_match_player_stats ADD COLUMN player_active_seconds INTEGER" + ) + if "active_time_source" not in columns: + connection.execute( + "ALTER TABLE rcon_match_player_stats ADD COLUMN active_time_source TEXT" + ) + + def _counter(player: dict[str, object], key: str) -> Counter[str]: value = player[key] if isinstance(value, Counter): diff --git a/backend/app/rcon_historical_read_model.py b/backend/app/rcon_historical_read_model.py index dd25c9e..13ff67a 100644 --- a/backend/app/rcon_historical_read_model.py +++ b/backend/app/rcon_historical_read_model.py @@ -5,6 +5,7 @@ from __future__ import annotations import json from datetime import datetime, timedelta, timezone +from .config import get_kpm_min_active_seconds from .historical_storage import ALL_SERVERS_SLUG from .normalizers import normalize_map_name from .player_external_profiles import build_external_player_profile_fields @@ -293,6 +294,12 @@ def _build_player_row( ) -> dict[str, object]: kills = _coerce_optional_int(row.get("kills")) or 0 deaths = _coerce_optional_int(row.get("deaths")) or 0 + player_active_seconds = _coerce_optional_int(row.get("player_active_seconds")) + active_time_payload = _build_player_active_time_payload( + kills=kills, + player_active_seconds=player_active_seconds, + active_time_source=row.get("active_time_source"), + ) return { "player_name": row.get("player_name"), "team": row.get("team"), @@ -303,10 +310,60 @@ def _build_player_row( "top_weapons": _top_counter(row.get("weapons_json")), "most_killed": _top_counter(row.get("most_killed_json")), "death_by": _top_counter(row.get("death_by_json")), + **active_time_payload, **build_external_player_profile_fields(player_id=row.get("player_id")), } +def _build_player_active_time_payload( + *, + kills: int, + player_active_seconds: int | None, + active_time_source: object, +) -> dict[str, object]: + min_active_seconds = get_kpm_min_active_seconds() + resolved_source = str(active_time_source or "").strip() or None + is_real_connection_time = resolved_source in { + "connection_intervals", + "connection_intervals_carryover", + } + if player_active_seconds is None: + return { + "player_active_seconds": None, + "player_active_minutes": None, + "kpm": None, + "kpm_status": "missing_active_time", + "active_time_source": resolved_source, + } + + player_active_minutes = round(player_active_seconds / 60, 3) + if not is_real_connection_time: + return { + "player_active_seconds": player_active_seconds, + "player_active_minutes": player_active_minutes, + "kpm": None, + "kpm_status": "missing_connection_intervals", + "active_time_source": resolved_source or "event_span_fallback", + } + if player_active_seconds < min_active_seconds: + return { + "player_active_seconds": player_active_seconds, + "player_active_minutes": player_active_minutes, + "kpm": None, + "kpm_status": "insufficient_active_time", + "active_time_source": resolved_source or "event_log", + } + + kpm = round(kills / (player_active_seconds / 60), 2) + return { + "player_active_seconds": player_active_seconds, + "player_active_minutes": player_active_minutes, + "kpm": kpm, + "kpm_status": "ready", + "active_time_source": resolved_source or "event_log", + } + + def _top_counter(raw_value: object, *, limit: int = 5) -> list[dict[str, object]]: if not isinstance(raw_value, str) or not raw_value.strip(): return [] diff --git a/backend/tests/test_rcon_materialization_pipeline.py b/backend/tests/test_rcon_materialization_pipeline.py index 5fd95d6..07533d8 100644 --- a/backend/tests/test_rcon_materialization_pipeline.py +++ b/backend/tests/test_rcon_materialization_pipeline.py @@ -4,6 +4,7 @@ from __future__ import annotations import gc import os +import sqlite3 import tempfile import unittest from pathlib import Path @@ -24,6 +25,7 @@ from app.rcon_admin_log_materialization import ( ) from app.rcon_admin_log_storage import persist_rcon_admin_log_entries from app.rcon_historical_read_model import ( + _build_player_active_time_payload, get_rcon_historical_match_detail, list_rcon_historical_recent_activity, ) @@ -58,6 +60,10 @@ class RconMaterializationPipelineTests(unittest.TestCase): self.assertEqual(players["Alpha"]["teamkills"], 1) self.assertEqual(players["Bravo"]["deaths"], 1) self.assertEqual(players["Charlie"]["deaths_by_teamkill"], 1) + self.assertEqual(players["Alpha"]["player_active_seconds"], 350) + self.assertEqual(players["Alpha"]["active_time_source"], "connection_intervals") + self.assertEqual(players["Bravo"]["player_active_seconds"], 0) + self.assertEqual(players["Bravo"]["active_time_source"], "event_span_fallback") self.assertEqual(status["materialized_matches"], 1) self.assertEqual(status["matches_with_player_stats"], 1) gc.collect() @@ -84,6 +90,11 @@ class RconMaterializationPipelineTests(unittest.TestCase): players = {row["player_name"]: row for row in detail["players"]} self.assertNotIn("player_id", players["Alpha"]) self.assertIn("kd_ratio", players["Alpha"]) + self.assertEqual(players["Alpha"]["player_active_seconds"], 350) + self.assertEqual(players["Alpha"]["kpm_status"], "ready") + self.assertEqual(players["Alpha"]["kpm"], 0.17) + self.assertEqual(players["Bravo"]["kpm_status"], "missing_connection_intervals") + self.assertIsNone(players["Bravo"]["kpm"]) self.assertEqual(players["Alpha"]["steam_id_64"], "76561198000000001") self.assertEqual(players["Alpha"]["platform"], "steam") self.assertEqual( @@ -95,6 +106,372 @@ class RconMaterializationPipelineTests(unittest.TestCase): self.assertEqual(players["Charlie"]["external_profile_links"], {}) gc.collect() + def test_materialization_migrates_existing_player_stats_schema_with_active_time_columns(self) -> None: + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as tmpdir: + db_path = Path(tmpdir) / "historical.sqlite3" + connection = sqlite3.connect(db_path) + try: + connection.executescript( + """ + CREATE TABLE rcon_admin_log_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + target_key TEXT NOT NULL, + external_server_id TEXT, + event_timestamp TEXT, + server_time INTEGER, + relative_time TEXT, + event_type TEXT NOT NULL, + raw_message TEXT NOT NULL, + canonical_message TEXT NOT NULL, + parsed_payload_json TEXT NOT NULL, + raw_entry_json TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + + CREATE TABLE rcon_materialized_matches ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + target_key TEXT NOT NULL, + external_server_id TEXT, + match_key TEXT NOT NULL, + map_name TEXT, + map_pretty_name TEXT, + game_mode TEXT, + started_server_time INTEGER, + ended_server_time INTEGER, + started_at TEXT, + ended_at TEXT, + allied_score INTEGER, + axis_score INTEGER, + winner TEXT, + confidence_mode TEXT NOT NULL, + source_basis TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(target_key, match_key) + ); + + CREATE TABLE rcon_match_player_stats ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + target_key TEXT NOT NULL, + match_key TEXT NOT NULL, + player_id TEXT NOT NULL, + player_name TEXT NOT NULL, + team TEXT, + kills INTEGER NOT NULL DEFAULT 0, + deaths INTEGER NOT NULL DEFAULT 0, + teamkills INTEGER NOT NULL DEFAULT 0, + deaths_by_teamkill INTEGER NOT NULL DEFAULT 0, + weapons_json TEXT NOT NULL DEFAULT '{}', + death_by_weapons_json TEXT NOT NULL DEFAULT '{}', + most_killed_json TEXT NOT NULL DEFAULT '{}', + death_by_json TEXT NOT NULL DEFAULT '{}', + first_seen_server_time INTEGER, + last_seen_server_time INTEGER, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(target_key, match_key, player_id) + ); + """ + ) + finally: + connection.close() + + materialize_rcon_admin_log(db_path=db_path) + + connection = sqlite3.connect(db_path) + try: + columns = { + row[1] + for row in connection.execute("PRAGMA table_info(rcon_match_player_stats)") + } + finally: + connection.close() + + self.assertIn("player_active_seconds", columns) + self.assertIn("active_time_source", columns) + gc.collect() + + def test_match_detail_keeps_kpm_missing_for_legacy_rows_without_active_time(self) -> None: + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as tmpdir: + db_path = Path(tmpdir) / "historical.sqlite3" + previous_storage_path = os.environ.get("HLL_BACKEND_STORAGE_PATH") + os.environ["HLL_BACKEND_STORAGE_PATH"] = str(db_path) + try: + materialize_rcon_admin_log(db_path=db_path) + connection = sqlite3.connect(db_path) + try: + connection.execute( + """ + INSERT INTO rcon_materialized_matches ( + target_key, external_server_id, match_key, map_name, map_pretty_name, + game_mode, started_server_time, ended_server_time, started_at, ended_at, + allied_score, axis_score, winner, confidence_mode, source_basis + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + "comunidad-hispana-01", + "comunidad-hispana-01", + "legacy-match", + "mortain_warfare", + "Mortain", + "warfare", + 100, + 500, + "2026-05-01T10:00:00Z", + "2026-05-01T11:00:00Z", + 5, + 3, + "allied", + "exact", + "admin-log-match-ended", + ), + ) + connection.execute( + """ + INSERT INTO rcon_match_player_stats ( + target_key, match_key, player_id, player_name, team, + kills, deaths, teamkills, deaths_by_teamkill, + weapons_json, death_by_weapons_json, most_killed_json, death_by_json, + first_seen_server_time, last_seen_server_time, player_active_seconds, active_time_source + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + """, + ( + "comunidad-hispana-01", + "legacy-match", + "steam-legacy", + "Legacy", + "Allies", + 10, + 5, + 0, + 0, + "{}", + "{}", + "{}", + "{}", + 120, + 480, + None, + None, + ), + ) + connection.commit() + finally: + connection.close() + detail = get_rcon_historical_match_detail( + server_key="comunidad-hispana-01", + match_id="legacy-match", + ) + finally: + _restore_env("HLL_BACKEND_STORAGE_PATH", previous_storage_path) + + self.assertIsNotNone(detail) + player = detail["players"][0] + self.assertIsNone(player["kpm"]) + self.assertEqual(player["kpm_status"], "missing_active_time") + gc.collect() + + def test_active_time_counts_full_match_for_player_connected_before_start(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T09:58:00Z", + "message": "[0 min (80)] CONNECTED Carry Over (steam-carry)", + }, + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:30:00Z", + "message": ( + "[31 min (1900)] KILL: Carry Over(Allies/steam-carry) -> " + "Victim(Axis/steam-victim) with M1 GARAND" + ), + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = {row["player_name"]: row for row in detail["players"]}["Carry Over"] + self.assertEqual(player["player_active_seconds"], 3600) + self.assertEqual(player["active_time_source"], "connection_intervals_carryover") + self.assertEqual(player["kpm_status"], "ready") + self.assertEqual(player["kpm"], 0.02) + + def test_active_time_counts_until_disconnect_for_player_connected_before_start(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T09:58:00Z", + "message": "[0 min (80)] CONNECTED Carry Over (steam-carry)", + }, + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:20:00Z", + "message": "[21 min (1300)] DISCONNECTED Carry Over (steam-carry)", + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = detail["players"][0] + self.assertEqual(player["player_active_seconds"], 1200) + self.assertEqual(player["active_time_source"], "connection_intervals_carryover") + + def test_active_time_counts_from_connect_until_match_end(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:10:00Z", + "message": "[11 min (700)] CONNECTED Late Join (steam-late)", + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = detail["players"][0] + self.assertEqual(player["player_active_seconds"], 3000) + self.assertEqual(player["active_time_source"], "connection_intervals") + + def test_active_time_sums_multiple_connection_intervals(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:05:00Z", + "message": "[6 min (400)] CONNECTED Reconnect (steam-reconnect)", + }, + { + "timestamp": "2026-05-01T10:15:00Z", + "message": "[16 min (1000)] DISCONNECTED Reconnect (steam-reconnect)", + }, + { + "timestamp": "2026-05-01T10:20:00Z", + "message": "[21 min (1300)] CONNECTED Reconnect (steam-reconnect)", + }, + { + "timestamp": "2026-05-01T10:35:00Z", + "message": "[36 min (2200)] DISCONNECTED Reconnect (steam-reconnect)", + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = detail["players"][0] + self.assertEqual(player["player_active_seconds"], 1500) + self.assertEqual(player["active_time_source"], "connection_intervals") + + def test_active_time_uses_event_span_fallback_without_ready_kpm_when_connection_intervals_missing(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:05:00Z", + "message": ( + "[6 min (400)] KILL: Fallback(Allies/steam-fallback) -> " + "Victim(Axis/steam-victim) with M1 GARAND" + ), + }, + { + "timestamp": "2026-05-01T10:20:00Z", + "message": "[21 min (1300)] CHAT[Team][Fallback(Allies/steam-fallback)]: test", + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = {row["player_name"]: row for row in detail["players"]}["Fallback"] + self.assertEqual(player["player_active_seconds"], 900) + self.assertEqual(player["active_time_source"], "event_span_fallback") + self.assertEqual(player["kpm_status"], "missing_connection_intervals") + self.assertIsNone(player["kpm"]) + + def test_kpm_is_null_when_active_time_is_below_threshold(self) -> None: + detail = _materialize_detail_from_entries( + entries=[ + { + "timestamp": "2026-05-01T10:00:00Z", + "message": "[1 min (100)] MATCH START Mortain Warfare", + }, + { + "timestamp": "2026-05-01T10:01:00Z", + "message": "[2 min (120)] CONNECTED Short (steam-short)", + }, + { + "timestamp": "2026-05-01T10:01:30Z", + "message": "[2 min (150)] DISCONNECTED Short (steam-short)", + }, + { + "timestamp": "2026-05-01T11:00:00Z", + "message": "[61 min (3700)] MATCH ENDED `Mortain Warfare` ALLIED (5 - 0) AXIS", + }, + ] + ) + + player = detail["players"][0] + self.assertEqual(player["player_active_seconds"], 30) + self.assertEqual(player["kpm_status"], "insufficient_active_time") + self.assertIsNone(player["kpm"]) + + def test_kpm_payload_helper_returns_ready_for_ten_kills_in_ten_minutes(self) -> None: + payload = _build_player_active_time_payload( + kills=10, + player_active_seconds=600, + active_time_source="connection_intervals", + ) + + self.assertEqual(payload["kpm"], 1.0) + self.assertEqual(payload["kpm_status"], "ready") + + def test_kpm_payload_helper_returns_zero_for_zero_kills_with_valid_active_time(self) -> None: + payload = _build_player_active_time_payload( + kills=0, + player_active_seconds=600, + active_time_source="connection_intervals", + ) + + self.assertEqual(payload["kpm"], 0.0) + self.assertEqual(payload["kpm_status"], "ready") + + def test_kpm_payload_helper_returns_missing_when_active_time_is_null(self) -> None: + payload = _build_player_active_time_payload( + kills=10, + player_active_seconds=None, + active_time_source="unavailable", + ) + + self.assertIsNone(payload["kpm"]) + self.assertEqual(payload["kpm_status"], "missing_active_time") + def test_match_detail_marks_equal_materialized_timestamps_as_server_time_only(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: db_path = Path(tmpdir) / "historical.sqlite3" @@ -455,6 +832,44 @@ def _persist_admin_log_fixture(db_path: Path) -> None: ) +def _materialize_detail_from_entries(*, entries: list[dict[str, object]]) -> dict[str, object]: + with tempfile.TemporaryDirectory(ignore_cleanup_errors=True) as tmpdir: + db_path = Path(tmpdir) / "historical.sqlite3" + previous_storage_path = os.environ.get("HLL_BACKEND_STORAGE_PATH") + os.environ["HLL_BACKEND_STORAGE_PATH"] = str(db_path) + try: + persist_rcon_admin_log_entries( + target={ + "target_key": "comunidad-hispana-01", + "external_server_id": "comunidad-hispana-01", + }, + entries=entries, + db_path=db_path, + ) + materialize_rcon_admin_log(db_path=db_path) + match_rows = list_materialized_rcon_matches_for_test(db_path) + detail = get_rcon_historical_match_detail( + server_key="comunidad-hispana-01", + match_id=str(match_rows[0]["match_key"]), + ) + finally: + _restore_env("HLL_BACKEND_STORAGE_PATH", previous_storage_path) + if detail is None: + raise AssertionError("expected materialized detail") + return detail + + +def list_materialized_rcon_matches_for_test(db_path: Path) -> list[dict[str, object]]: + from app.rcon_admin_log_materialization import list_materialized_rcon_matches + + return list_materialized_rcon_matches( + target_key="comunidad-hispana-01", + only_ended=True, + limit=5, + db_path=db_path, + ) + + def _persist_scoreboard_match(db_path: Path) -> None: upsert_historical_match( server_slug="comunidad-hispana-01", diff --git a/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md b/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md index abbe408..8300832 100644 --- a/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md +++ b/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md @@ -2,94 +2,166 @@ ## Scope -This analysis covers the player table inside `historico-partida.html` for historical match detail. +This document defines how real historical KPM works for the match detail served to `historico-partida.html`. -## Question +## Why KPM Was Not Safe Before -Can the UI add a `Kills por minuto` column now without reintroducing false KPM? - -## Short Answer - -No. - -## Why It Cannot Be Implemented Honestly Yet - -The current historical match detail payload exposes player combat totals and match-level timing, but it does not expose a trustworthy player-level active time. - -Available today: +Historical match detail already exposed: - per player: - `kills` - `deaths` - `teamkills` - - `kd_ratio` or enough data to derive it - - `top_weapons` - - `most_killed` - - `death_by` + - weapon and matchup counters - per match: - - `duration_seconds` - `started_at` - `ended_at` + - `duration_seconds` -Missing for real KPM: +What it did not expose was player-level active time. Because of that, dividing by total match duration would have produced a false player KPM. -- `player_active_seconds` -- real played minutes per player -- presence intervals -- a quality flag that tells whether playtime is exact, observed, estimated or unknown +## Real KPM Rule -## Data Review - -`build_historical_match_detail_payload()` serves `item` from the RCON historical read model when available. - -`historico-partida.js` currently renders the player table with: - -- `Jugador` -- `Equipo` -- `Kills` -- `Muertes` -- `TK` -- `KD` - -There is no frontend or payload field for honest KPM. - -## Important Constraint - -Real KPM must mean: +Real KPM means: ```text kills / (player_active_seconds / 60) ``` -It must not mean: +It does not mean: ```text kills / match_duration_minutes ``` -unless the product intentionally introduces a different metric with a clearly different label. +## Source of Truth -## Existing Design Reference +The current implementation uses connection intervals reconstructed from the materialized RCON AdminLog match model. -`docs/REAL_KPM_PLAYER_ACTIVE_SECONDS_DESIGN.md` already documents the correct long-term direction: +Reliable interval signals: -- materialize `player_active_seconds` -- carry a playtime quality classification -- compute KPM in backend read models and snapshots -- never compute real KPM in frontend JavaScript +- `connected` +- `disconnected` +- `match_start` +- `match_end` -## Recommendation +Supporting evidence still stored in the player fact: -Do not add the column now. +- `first_seen_server_time` +- `last_seen_server_time` -Safe next step: +The persisted field is: -1. Add real `player_active_seconds` to the historical player detail model. -2. Expose it in the match detail payload. -3. Add `Kills por minuto` only when backed by that field. +- `player_active_seconds` -Optional weaker alternative, only with explicit product approval: +The source label is: -- `Kills/min partida` -- clearly labeled as match-duration-based -- never presented as player KPM +- `active_time_source = "event_log"` + +## Persistence + +Forward-only persistence now stores on `rcon_match_player_stats`: + +- `player_active_seconds INTEGER NULL` +- `active_time_source TEXT` + +Legacy rows remain valid. If they were materialized before these columns existed, they keep `player_active_seconds = NULL` until new materialization or new matches populate the field. + +## Calculation + +Observed active time is now: + +```text +sum(connected_interval_seconds clamped to [match_start, match_end]) +``` + +Rules: + +- if the player connects during the match: + - open interval at `connected.server_time` +- if the player disconnects during the match: + - close interval at `disconnected.server_time` +- if the player was already connected before `match_start` and there is no later pre-match disconnect: + - open interval at `match_start` +- if the player is still connected at `match_end`: + - close interval at `match_end` +- if the player reconnects multiple times: + - sum all non-overlapping intervals + +This is still observed presence, not exact telemetry down to every silent second. + +KPM is exposed only when: + +- `player_active_seconds` exists +- `player_active_seconds >= HLL_KPM_MIN_ACTIVE_SECONDS` + +Default: + +- `HLL_KPM_MIN_ACTIVE_SECONDS = 60` + +## Payload Contract + +Historical match detail player rows may now expose: + +- `player_active_seconds` +- `player_active_minutes` +- `kpm` +- `kpm_status` +- `active_time_source` + +`active_time_source` values currently used: + +- `connection_intervals` +- `connection_intervals_carryover` +- `event_span_fallback` +- `unavailable` + +`kpm_status` values: + +- `ready` +- `missing_active_time` +- `insufficient_active_time` +- `missing_connection_intervals` + +Rules: + +- missing active time: + - `kpm = null` + - `kpm_status = "missing_active_time"` +- fallback event span without reliable connection intervals: + - `kpm = null` + - `kpm_status = "missing_connection_intervals"` +- active time below threshold: + - `kpm = null` + - `kpm_status = "insufficient_active_time"` +- valid active time: + - `kpm = round(kills / (player_active_seconds / 60), 2)` + - `kpm_status = "ready"` + - only when `active_time_source` is `connection_intervals` or `connection_intervals_carryover` + +## Historical Matches Already Stored + +Old matches are not backfilled with fake KPM. + +For those rows: + +- `player_active_seconds` can remain `null` +- `kpm` stays `null` +- frontend must not render `0.00` as if the value were real +- rows rematerialized without reliable connection intervals can still expose + `event_span_fallback`, but that fallback must not be shown as real KPM + +## Frontend Rule + +`historico-partida.js` renders KPM only when: + +- `kpm_status == "ready"` + +If the value is missing or below threshold, the panel stays clean and does not show a fake metric. + +## Limitations + +- This is observed active time from AdminLog connection evidence, not exact join/leave telemetry for every silent second. +- Quiet players with kills/chat/team switches but no reliable connection chain can expose `event_span_fallback`; that span is intentionally blocked from KPM. +- Legacy matches remain without KPM unless rematerialized from stored AdminLog evidence. +- We do not discount time spent without squad/unit/role yet because there is no audited historical source for that dimension in this implementation. diff --git a/frontend/assets/js/historico-partida.js b/frontend/assets/js/historico-partida.js index dcfa066..c1f9a2b 100644 --- a/frontend/assets/js/historico-partida.js +++ b/frontend/assets/js/historico-partida.js @@ -507,6 +507,10 @@ function isInactiveMatchPlayer(player) { function renderPlayerStatsPanel(player, item, context) { const matchups = buildPlayerDirectMatchups(player); + const kpmChip = + player?.kpm_status === "ready" + ? renderPlayerStatChip("KPM", formatDecimal(player.kpm, 2)) + : ""; const hasExpandedStats = hasNamedCounts(player.top_weapons) || hasNamedCounts(player.most_killed) || @@ -525,6 +529,7 @@ function renderPlayerStatsPanel(player, item, context) { ${renderPlayerStatChip("Muertes", formatOptionalNumber(player.deaths))} ${renderPlayerStatChip("TK", formatOptionalNumber(player.teamkills))} ${renderPlayerStatChip("KD", formatKdRatio(player))} + ${kpmChip} ${renderExternalProfilesSection(player)}