diff --git a/ai/tasks/done/TASK-254-fix-historical-ratio-values-and-stats-profile-performance-regression.md b/ai/tasks/done/TASK-254-fix-historical-ratio-values-and-stats-profile-performance-regression.md new file mode 100644 index 0000000..8f56229 --- /dev/null +++ b/ai/tasks/done/TASK-254-fix-historical-ratio-values-and-stats-profile-performance-regression.md @@ -0,0 +1,89 @@ +# TASK-254 - Fix historical ratio values and stats profile performance regression + +## Summary + +This task fixes two regressions detected before pushing TASK-252 and TASK-253: + +1. Historical leaderboard ratio columns could show `0,00` when the direct ratio field was `null`. +2. The public player profile in `stats.html` could become slow when the weekly/monthly read model was empty and the endpoint fell back to runtime aggregation. + +## Historical Ratios + +Root cause: + +- `formatHistoricalPerMatch(...)` converted `null` direct values with `Number(null)`, which becomes `0`. +- For `Top muertes` and `Soporte`, that caused the formatter to stop early and render `0,00` instead of dividing the real total by matches. + +Fix: + +- treat `null`, `undefined` and empty string as missing direct values +- only use a direct ratio value when it is truly present +- otherwise compute: + - kills / partidas + - muertes / partidas + - soporte / partidas +- if `matches <= 0` or the total is missing, render an empty string instead of a fake `0,00` + +Expected examples: + +- kills `170`, matches `4` -> `42,50` +- deaths `225`, matches `14` -> `16,07` + +## Stats Profile Performance + +Root cause: + +- TASK-253 added a runtime fallback for weekly/monthly personal profile reads when `player_period_stats` was empty. +- That fallback queried live materialized match/player tables and ranking windows on the request path. +- For public profile reads this is too expensive and can degrade page responsiveness. + +Fix: + +- disable the runtime fallback for weekly/monthly profile reads +- keep the endpoint read-model-first only for public weekly/monthly profile windows +- when the read model is missing: + - return a lightweight profile payload + - keep external profile links and platform identity + - keep `kpm = null` + - keep `kpm_status = missing_active_time` + +This preserves: + +- translated labels +- hidden personal ID in the visible profile card +- external profile buttons +- no fake KPM + +## Teamkills + +No TeamKills change was implemented here. + +Reference: + +- `TASK-251-investigate-teamkills-ranking-zeroes.md` + +## Validation + +Executed: + +- `node --check frontend/assets/js/historico.js` +- `node --check frontend/assets/js/stats.js` +- `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` + +Static/manual checks: + +- no `undefined` titles remain in historical leaderboard labels +- `Kills/partida` only stays on `Top kills` +- `Muertes/partida` stays on `Top muertes` +- `matches_over_100_kills` keeps no ratio column +- Steam profile buttons still resolve to Steam + Hellor + HLL Records + Helo +- Epic profile buttons still resolve to Hellor + HLL Records + +## Notes + +- No backend 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 973562b..8e77d40 100644 --- a/backend/app/rcon_historical_player_stats.py +++ b/backend/app/rcon_historical_player_stats.py @@ -378,18 +378,12 @@ def get_rcon_materialized_player_stats( if read_model_result is not None: return read_model_result - runtime_result = _get_rcon_materialized_player_stats_runtime( + return _build_missing_player_period_stats_result( player_id=normalized_player_id, server_id=server_id, timeframe=resolved_timeframe, - db_path=db_path, + missing_reason=fallback_reason or "player-period-stats-unavailable", ) - runtime_result.setdefault("source", {}) - runtime_result["source"]["fallback_used"] = True - runtime_result["source"]["fallback_reason"] = fallback_reason or "player-period-stats-unavailable" - runtime_result["source"]["read_model"] = "player-period-stats" - runtime_result["source"]["freshness"] = "runtime-fallback" - return runtime_result return _get_rcon_materialized_player_stats_runtime( player_id=normalized_player_id, @@ -1655,6 +1649,21 @@ def _build_missing_player_period_stats_result( "kills": 0, "deaths": 0, "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": 0, + "eligible_kills": 0, + "minimum_active_seconds": get_kpm_min_active_seconds(), + "sources": [], + }, + **build_external_player_profile_fields(player_id=player_id), "weekly_ranking": None, "monthly_ranking": None, "source": { diff --git a/backend/tests/test_stats_player_profile_payload.py b/backend/tests/test_stats_player_profile_payload.py index af64882..39458cf 100644 --- a/backend/tests/test_stats_player_profile_payload.py +++ b/backend/tests/test_stats_player_profile_payload.py @@ -11,40 +11,22 @@ from app.rcon_historical_player_stats import ( class StatsPlayerProfilePayloadTests(unittest.TestCase): - def test_player_stats_falls_back_to_runtime_when_period_read_model_is_empty(self) -> None: - runtime_payload = { - "player_id": "76561198000000000", - "player_name": "Runtime Player", - "server_id": "all-servers", - "timeframe": "weekly", - "window_kind": "current-week", - "matches_considered": 3, - "kills": 25, - "deaths": 12, - "teamkills": 1, - "weekly_ranking": {"metric": "kills", "ranking_position": 5}, - "monthly_ranking": {"metric": "kills", "ranking_position": 8}, - "source": {"primary_source": "rcon"}, - } - with ( - patch( - "app.rcon_historical_player_stats._get_player_period_stats_read_model", - return_value=(None, "player-period-stats-empty"), - ), - patch( - "app.rcon_historical_player_stats._get_rcon_materialized_player_stats_runtime", - return_value=runtime_payload, - ) as runtime_loader, + 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", + return_value=(None, "player-period-stats-empty"), ): result = get_rcon_materialized_player_stats( player_id="76561198000000000", timeframe="weekly", ) - runtime_loader.assert_called_once() - self.assertTrue(result["source"]["fallback_used"]) - self.assertEqual(result["source"]["fallback_reason"], "player-period-stats-empty") - self.assertEqual(result["source"]["freshness"], "runtime-fallback") + self.assertEqual(result["matches_considered"], 0) + self.assertEqual(result["kpm_status"], "missing_active_time") + self.assertEqual(result["platform"], "steam") + self.assertIn("steam", result["external_profile_links"]) + self.assertEqual(result["source"]["missing_reason"], "player-period-stats-empty") + self.assertFalse(result["source"]["fallback_used"]) def test_build_stats_player_profile_payload_exposes_ready_real_kpm_and_external_links(self) -> None: with patch( diff --git a/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md b/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md index 380124a..20d2fe0 100644 --- a/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md +++ b/docs/HISTORICAL_MATCH_KILLS_PER_MINUTE_ANALYSIS.md @@ -176,7 +176,11 @@ This avoids false `0.00` values for: ## Personal Player Profile -The public player profile in `stats.html` can now expose real KPM for the selected weekly or monthly window when the profile endpoint can aggregate: +The public player profile in `stats.html` can now expose real KPM for the selected weekly or monthly window when the lightweight player-period read model already contains enough data for that player and scope. + +The profile endpoint does not use a runtime fallback over `rcon_match_player_stats` anymore for weekly/monthly public reads. That fallback was disabled because it could degrade public profile response times when the read model was empty or stale. + +When the read model is available, profile KPM uses: - `SUM(player_active_seconds)` - only over rows with: @@ -195,7 +199,7 @@ This is intentionally narrower than the window total: - profile `kills` still show all kills in the selected window - profile `KPM` only becomes `ready` when the active-time subset is reliable enough -The profile payload now exposes: +The profile payload exposes: - `player_active_seconds` - `player_active_minutes` @@ -204,7 +208,14 @@ The profile payload now exposes: - `active_time_source` - `active_time_coverage` -If the profile read model is empty, the endpoint can safely fall back to runtime aggregation over `rcon_match_player_stats` for that single player and time window. This avoids inventing KPM while keeping the page usable before the read model is refreshed. +If the profile read model is empty or unavailable: + +- the endpoint keeps the profile fast +- `kpm` stays `null` +- `kpm_status` stays `missing_active_time` +- external links and profile identity fields remain available + +This keeps the page responsive without inventing KPM. ## Kills Per Match Labels diff --git a/frontend/assets/js/historico.js b/frontend/assets/js/historico.js index fc1e558..f9afe8d 100644 --- a/frontend/assets/js/historico.js +++ b/frontend/assets/js/historico.js @@ -1783,18 +1783,24 @@ function formatHistoricalRatio(item, metricConfig, matches) { } function formatHistoricalPerMatch(rawDirectValue, rawTotalValue, rawMatches) { - const directValue = Number(rawDirectValue); + const directValue = + rawDirectValue === null || rawDirectValue === undefined || rawDirectValue === "" + ? Number.NaN + : Number(rawDirectValue); if (Number.isFinite(directValue)) { return formatDecimal(directValue, 2); } - const kills = Number(rawTotalValue); + const totalValue = + rawTotalValue === null || rawTotalValue === undefined || rawTotalValue === "" + ? Number.NaN + : Number(rawTotalValue); const matches = Number(rawMatches); - if (!Number.isFinite(kills) || !Number.isFinite(matches) || matches <= 0) { - return "-"; + if (!Number.isFinite(totalValue) || !Number.isFinite(matches) || matches <= 0) { + return ""; } - return formatDecimal(kills / matches, 2); + return formatDecimal(totalValue / matches, 2); } function formatDateOnly(timestamp) {