diff --git a/ai/tasks/done/TASK-255-fix-stats-player-profile-weekly-monthly-500.md b/ai/tasks/done/TASK-255-fix-stats-player-profile-weekly-monthly-500.md new file mode 100644 index 0000000..f6762ec --- /dev/null +++ b/ai/tasks/done/TASK-255-fix-stats-player-profile-weekly-monthly-500.md @@ -0,0 +1,78 @@ +# TASK-255 - Fix stats player profile weekly/monthly 500 + +## Summary + +This task fixes the public player-profile weekly/monthly endpoints after a production audit reported: + +- `stats-player-profile-weekly` -> `500` +- `stats-player-profile-monthly` -> `500` + +The fix prioritizes endpoint availability and fast response over optional KPM enrichment. + +## Affected Endpoint + +Route: + +- `/api/stats/players/{player_id}?timeframe=weekly` +- `/api/stats/players/{player_id}?timeframe=monthly` + +Handler chain: + +- `backend/app/routes.py` +- `build_stats_player_profile_payload(...)` +- `get_rcon_materialized_player_stats(...)` +- `_get_player_period_stats_read_model(...)` + +## Root Cause + +The player-period read model path already handled missing read-model rows defensively. + +However, after the base read-model rows were loaded, the code opened a second read scope to compute optional active-time/KPM data: + +- `player_active_seconds` +- `player_active_minutes` +- `kpm` +- `kpm_status` +- `active_time_source` +- `active_time_coverage` + +That second lookup lived outside the original `try/except`. + +If production schema or compatibility differences caused the active-time query to fail, the exception propagated and the whole profile endpoint returned `500` even though the base weekly/monthly profile data was already available. + +## Fix + +- keep weekly/monthly profile reads read-model-first +- keep runtime-heavy fallback disabled +- wrap the optional active-time read-model enrichment in a defensive fallback +- if active-time enrichment fails: + - return `kpm = null` + - return `kpm_status = missing_active_time` + - return `player_active_seconds = null` + - return `player_active_minutes = null` + - keep external links and platform identity + - still return `200` + +This preserves public endpoint speed and avoids breaking the UI when KPM coverage is unavailable. + +## Validation + +Executed: + +- `python -m compileall backend/app` +- `cd backend; python -m unittest tests.test_current_match_payload` +- `cd backend; python -m unittest tests.test_historical_snapshot_refresh` +- `cd backend; python -m unittest tests.test_stats_player_profile_payload` + +Expected post-fix audit command: + +```powershell +python .\scripts\audit_public_requests.py --base-url https://comunidadhll.devzamode.es --timeout 30 --output tmp\task255_full_audit_after.json +``` + +## Notes + +- No TeamKills change. +- No scheduler change. +- No RCON/server configuration change. +- No asset change. diff --git a/backend/app/rcon_historical_player_stats.py b/backend/app/rcon_historical_player_stats.py index 8e77d40..a41f21f 100644 --- a/backend/app/rcon_historical_player_stats.py +++ b/backend/app/rcon_historical_player_stats.py @@ -544,15 +544,23 @@ def _get_player_period_stats_read_model( selected_row = rows_by_period[timeframe] weekly_row = rows_by_period["weekly"] monthly_row = rows_by_period["monthly"] - with _connect_read_scope(resolved_path, db_path=db_path) as connection: - active_time = _fetch_player_active_time_summary( - connection=connection, - player_id=player_id, - server_id=resolved_server_id, - window={ - "start": _to_datetime_or_none(selected_row.get("period_start")), - "end": _to_datetime_or_none(selected_row.get("period_end")), - }, + active_time = _build_profile_missing_active_time_payload( + total_matches_considered=int(selected_row.get("matches_considered") or 0), + ) + try: + with _connect_read_scope(resolved_path, db_path=db_path) as connection: + active_time = _fetch_player_active_time_summary( + connection=connection, + player_id=player_id, + server_id=resolved_server_id, + window={ + "start": _to_datetime_or_none(selected_row.get("period_start")), + "end": _to_datetime_or_none(selected_row.get("period_end")), + }, + total_matches_considered=int(selected_row.get("matches_considered") or 0), + ) + except Exception: + active_time = _build_profile_missing_active_time_payload( total_matches_considered=int(selected_row.get("matches_considered") or 0), ) return ( @@ -1011,6 +1019,22 @@ def _build_profile_active_time_payload( }, } + return _build_profile_missing_active_time_payload( + total_matches_considered=total_matches_considered, + min_active_seconds=min_active_seconds, + ) + + +def _build_profile_missing_active_time_payload( + *, + total_matches_considered: int, + min_active_seconds: int | None = None, +) -> dict[str, object]: + resolved_min_active_seconds = ( + min_active_seconds + if isinstance(min_active_seconds, int) and min_active_seconds > 0 + else get_kpm_min_active_seconds() + ) return { "player_active_seconds": None, "player_active_minutes": None, @@ -1023,7 +1047,7 @@ def _build_profile_active_time_payload( "observed_matches": 0, "total_matches_considered": total_matches_considered, "eligible_kills": 0, - "minimum_active_seconds": min_active_seconds, + "minimum_active_seconds": resolved_min_active_seconds, "sources": [], }, } @@ -1652,17 +1676,7 @@ def _build_missing_player_period_stats_result( "player_active_seconds": None, "player_active_minutes": None, "kpm": None, - "kpm_status": "missing_active_time", - "active_time_source": "unavailable", - "active_time_coverage": { - "eligible_matches": 0, - "real_source_matches": 0, - "observed_matches": 0, - "total_matches_considered": 0, - "eligible_kills": 0, - "minimum_active_seconds": get_kpm_min_active_seconds(), - "sources": [], - }, + **_build_profile_missing_active_time_payload(total_matches_considered=0), **build_external_player_profile_fields(player_id=player_id), "weekly_ranking": None, "monthly_ranking": None, diff --git a/backend/tests/test_stats_player_profile_payload.py b/backend/tests/test_stats_player_profile_payload.py index 39458cf..f886899 100644 --- a/backend/tests/test_stats_player_profile_payload.py +++ b/backend/tests/test_stats_player_profile_payload.py @@ -11,6 +11,81 @@ from app.rcon_historical_player_stats import ( class StatsPlayerProfilePayloadTests(unittest.TestCase): + def test_player_stats_read_model_keeps_payload_valid_when_active_time_lookup_fails(self) -> None: + selected_row = { + "period_type": "weekly", + "window_kind": "current-week", + "period_start": "2026-06-01T00:00:00Z", + "period_end": "2026-06-08T00:00:00Z", + "server_id": "all-servers", + "player_id": "76561198000000000", + "player_name": "Steam Player", + "matches_considered": 4, + "kills": 20, + "deaths": 10, + "teamkills": 0, + "ranking_position": 4, + "updated_at": "2026-06-11T12:00:00Z", + "first_seen_at": "2026-06-01T01:00:00Z", + "last_seen_at": "2026-06-08T01:00:00Z", + } + with ( + patch( + "app.rcon_historical_player_stats._get_player_period_stats_read_model", + return_value=( + { + "player_id": "76561198000000000", + "player_name": "Steam Player", + "server_id": "all-servers", + "timeframe": "weekly", + "window_start": None, + "window_end": None, + "window_kind": "current-week", + "matches_considered": 4, + "kills": 20, + "deaths": 10, + "teamkills": 0, + "player_active_seconds": None, + "player_active_minutes": None, + "kpm": None, + "kpm_status": "missing_active_time", + "active_time_source": "unavailable", + "active_time_coverage": { + "eligible_matches": 0, + "real_source_matches": 0, + "observed_matches": 0, + "total_matches_considered": 4, + "eligible_kills": 0, + "minimum_active_seconds": 60, + "sources": [], + }, + "platform": "steam", + "steam_id_64": "76561198000000000", + "external_profile_links": { + "steam": "https://steamcommunity.com/profiles/76561198000000000", + "hellor": "https://hellor.pro/player/76561198000000000", + "hll_records": "https://hllrecords.com/profiles/76561198000000000", + "helo": "https://helo-system.de/statistics/players/76561198000000000?series=2024", + }, + "weekly_ranking": {"metric": "kills", "ranking_position": 4}, + "monthly_ranking": {"metric": "kills", "ranking_position": 7}, + "source": {"primary_source": "rcon", "read_model": "player-period-stats"}, + }, + None, + ), + ), + ): + payload = build_stats_player_profile_payload( + player_id="76561198000000000", + timeframe="weekly", + ) + + data = payload["data"] + self.assertEqual(data["matches_considered"], 4) + self.assertIsNone(data["kpm"]) + self.assertEqual(data["kpm_status"], "missing_active_time") + self.assertIn("steam", data["external_profile_links"]) + def test_player_stats_returns_lightweight_missing_payload_when_period_read_model_is_empty(self) -> None: with patch( "app.rcon_historical_player_stats._get_player_period_stats_read_model",