Optimize weekly monthly ranking read path

ai/tasks/done/TASK-215-optimize-weekly-monthly-ranking-read-path.md

@@ -0,0 +1,312 @@
+---
+id: TASK-215
+title: Optimize weekly and monthly ranking read path
+status: done
+type: backend
+team: Backend Senior
+supporting_teams:
+  - Arquitecto Python
+roadmap_item: foundation
+priority: high
+---
+
+# TASK-215 - Optimize weekly and monthly ranking read path
+
+## Goal
+
+Optimizar la ruta publica de lectura de los rankings weekly y monthly para que lean exclusivamente snapshots ya materializados, evitando inicializacion, migracion o setup de storage en cada request publica y reduciendo la latencia del read path sin tocar frontend, assets ni Elo/MMR.
+
+## Context
+
+Tras `TASK-211` se corrigio la ruta anual para separar lectura publica de generacion/escritura. Ahora se ha detectado el mismo patron en weekly/monthly.
+
+Evidencia de produccion:
+
+- la API publica devuelve correctamente:
+  - `snapshot_status = ready`
+  - `read_model = ranking-snapshot`
+  - `fallback_used = false`
+  - `items = 20`
+- el perfilado directo de:
+
+```python
+build_global_ranking_payload(
+    timeframe="weekly",
+    metric="kills",
+    limit=20,
+    server_id="all",
+)
+```
+
+ha tardado `101.33 segundos`.
+
+`cProfile` indica:
+
+- `payloads.py:787 build_global_ranking_payload -> 101.330s`
+- `rcon_historical_leaderboards.py:315 get_latest_ranking_snapshot -> 101.330s`
+- `rcon_historical_leaderboards.py:98 initialize_ranking_snapshot_storage -> 101.306s`
+- `rcon_admin_log_materialization.py:24 initialize_rcon_materialized_storage -> 101.294s`
+- `postgres_rcon_storage.py:383 initialize_postgres_rcon_storage -> 101.315s`
+
+La causa actual es que `get_latest_ranking_snapshot()` llama a `initialize_ranking_snapshot_storage()` dentro de una ruta publica de solo lectura. Esa funcion inicializa o migra storage y no debe ejecutarse durante una request publica que solo lee snapshots ya materializados.
+
+Regla arquitectonica:
+
+- los endpoints publicos de ranking weekly/monthly deben leer unicamente:
+  - `ranking_snapshots`
+  - `ranking_snapshot_items`
+- no deben:
+  - consultar RCON
+  - consultar scoreboard publico
+  - recalcular rankings en runtime
+  - escanear `rcon_match_player_stats`
+  - inicializar o migrar storage en request publico
+  - hacer N+1 por jugador
+  - preparar fallback runtime salvo modo interno explicito
+
+Preserve the current product identity: Spanish-speaking HLL Vietnam community, military/Vietnam/tactical/sober visual direction and controlled repository evolution.
+
+## Steps
+
+1. Revisar primero la implementacion actual en `backend/app/rcon_historical_leaderboards.py` y el ensamblado de payloads en `backend/app/payloads.py`.
+2. Tomar como referencia conceptual la separacion aplicada en `TASK-211` para annual.
+3. Separar con claridad:
+   - inicializacion, generacion o escritura de snapshots
+   - lectura publica de snapshots
+4. Mantener `initialize_ranking_snapshot_storage()` disponible para:
+   - generacion
+   - refresh
+   - CLI
+   - procesos internos
+   - SQLite legacy si realmente lo requiere
+5. Evitar `initialize_ranking_snapshot_storage()` en `get_latest_ranking_snapshot()` cuando se use PostgreSQL por defecto.
+6. En la ruta PostgreSQL publica:
+   - abrir conexion directa con `connect_postgres_compat()`
+   - buscar snapshot en `ranking_snapshots`
+   - contar o listar `ranking_snapshot_items`
+   - devolver payload sin inicializar storage
+7. Mantener compatibilidad SQLite legacy explicita:
+   - si `db_path` se pasa explicitamente, resolver path de solo lectura sin migrar o inicializar si el diseno actual lo permite
+   - no romper tests existentes
+8. Mantener el contrato de respuesta:
+   - `read_model = ranking-snapshot`
+   - `snapshot_status = ready`
+   - `fallback_used = false`
+   - `items` segun `limit`
+   - weekly/monthly siguen funcionando para `all`, `comunidad-hispana-01` y `comunidad-hispana-02`
+   - las metricas existentes siguen funcionando
+9. Anadir un test unitario con monkeypatch o mock para verificar que `get_latest_ranking_snapshot()` no invoca `initialize_ranking_snapshot_storage()` en la ruta PostgreSQL de lectura publica, tomando como referencia el test creado en `TASK-211`.
+
+## Files to Read First
+
+- `AGENTS.md`
+- `ai/repo-context.md`
+- `ai/architecture-index.md`
+- `backend/app/rcon_historical_leaderboards.py`
+- `backend/app/payloads.py`
+- `ai/tasks/done/TASK-211-optimize-annual-ranking-read-path.md`
+
+## Expected Files to Modify
+
+- `ai/tasks/in-progress/TASK-215-optimize-weekly-monthly-ranking-read-path.md`
+- `backend/app/rcon_historical_leaderboards.py`
+- `backend/app/payloads.py`
+
+Optional only if strictly necessary:
+
+- backend unit test file related to ranking snapshots or payloads
+
+## Constraints
+
+- Keep the change minimal.
+- Preserve HLL Vietnam project identity.
+- Do not introduce unnecessary frameworks or dependencies.
+- Do not overwrite repository-specific context with generic platform template text.
+- No ejecutar `ai-platform run`.
+- No tocar `frontend/`.
+- No tocar assets de armas.
+- No tocar SVGs.
+- No modificar imagenes fisicas.
+- No reactivar Elo/MMR.
+- No reintroducir Comunidad Hispana #03.
+- No tocar `ai/system-metrics.md`.
+- No hacer push.
+- No consultar RCON, scoreboard publico ni recalcular ranking dentro de la lectura publica weekly/monthly.
+- No escanear `rcon_match_player_stats` en la ruta publica weekly/monthly.
+- No hacer N+1 por jugador en la ruta publica weekly/monthly.
+- No inicializar ni migrar storage en request publico weekly/monthly.
+- Los endpoints publicos deben leer read models propios en PostgreSQL.
+- La lectura publica weekly/monthly debe leer exclusivamente `ranking_snapshots` y `ranking_snapshot_items`.
+- Mantener compatibilidad con SQLite legacy solo como modo explicito si ya existe soporte real para ello.
+
+## Validation
+
+Before completing the task ensure:
+
+- `get_latest_ranking_snapshot()` no invoca `initialize_ranking_snapshot_storage()` en la ruta PostgreSQL de lectura publica
+- no aparece `initialize_ranking_snapshot_storage` en el perfil de la ruta publica
+- no aparece `initialize_rcon_materialized_storage` en el perfil de la ruta publica
+- no aparece `initialize_postgres_rcon_storage` en el perfil de la ruta publica
+- la generacion o escritura de snapshots sigue pudiendo inicializar storage cuando sea necesario
+- la lectura publica weekly/monthly no recalcula ranking ni cae a runtime aggregates
+- la respuesta mantiene:
+  - `read_model = ranking-snapshot`
+  - `snapshot_status = ready`
+  - `fallback_used = false`
+  - `items` segun `limit`
+- se ejecuta `compileall`:
+
+```bash
+python -m compileall backend\app\rcon_historical_leaderboards.py backend\tests\<test_file>.py
+```
+
+- se ejecuta el test unitario anadido
+- se ejecuta esta medicion directa:
+
+```bash
+python - <<'PY'
+import time
+from app.payloads import build_global_ranking_payload
+
+tests = [
+    {"timeframe": "weekly", "metric": "kills", "limit": 20, "server_id": "all"},
+    {"timeframe": "weekly", "metric": "kills", "limit": 30, "server_id": "all"},
+    {"timeframe": "monthly", "metric": "kills", "limit": 20, "server_id": "all"},
+]
+
+for params in tests:
+    for i in range(3):
+        start = time.perf_counter()
+        payload = build_global_ranking_payload(**params)
+        elapsed = time.perf_counter() - start
+        data = payload.get("data", payload)
+        print({
+            "params": params,
+            "attempt": i + 1,
+            "seconds": round(elapsed, 3),
+            "items": len(data.get("items") or []),
+            "snapshot_status": data.get("snapshot_status"),
+            "read_model": (data.get("source") or {}).get("read_model"),
+            "fallback_used": data.get("fallback_used"),
+        })
+PY
+```
+
+- si hay backend local accesible, se ejecuta esta medicion HTTP interna:
+
+```bash
+python - <<'PY'
+import json
+import time
+from urllib.request import urlopen
+
+urls = [
+    "http://127.0.0.1:8000/api/ranking?timeframe=weekly&metric=kills&limit=20",
+    "http://127.0.0.1:8000/api/ranking?timeframe=weekly&metric=kills&limit=30",
+    "http://127.0.0.1:8000/api/ranking?timeframe=monthly&metric=kills&limit=20",
+]
+
+for url in urls:
+    for i in range(3):
+        start = time.perf_counter()
+        with urlopen(url, timeout=30) as response:
+            body = response.read()
+        elapsed = time.perf_counter() - start
+        payload = json.loads(body.decode("utf-8"))
+        data = payload.get("data", {})
+        print({
+            "url": url,
+            "attempt": i + 1,
+            "seconds": round(elapsed, 3),
+            "http": response.status,
+            "items": len(data.get("items") or []),
+            "snapshot_status": data.get("snapshot_status"),
+            "read_model": (data.get("source") or {}).get("read_model"),
+            "fallback_used": data.get("fallback_used"),
+        })
+PY
+```
+
+- weekly top 20 queda por debajo de `1 segundo`
+- idealmente weekly top 20 queda por debajo de `300 ms`
+- weekly/monthly siguen funcionando para `all`, `comunidad-hispana-01` y `comunidad-hispana-02`
+- las metricas existentes siguen funcionando
+- `git diff --name-only` matches the expected scope
+- no unrelated files were modified
+
+## Outcome
+
+Result:
+
+- Updated `backend/app/rcon_historical_leaderboards.py` to split public weekly/monthly snapshot reads from storage initialization and snapshot generation.
+- Public reads now use a dedicated read-only connection helper:
+  - PostgreSQL default path opens a direct compat read connection without `initialize_ranking_snapshot_storage()`
+  - explicit SQLite legacy path resolves the file in read-only mode without migrations or setup
+- Kept `initialize_ranking_snapshot_storage()` for generation, refresh, CLI and internal write flows.
+- Tightened public ranking behavior so runtime fallback is no longer enabled by default; public requests only fall back when `HLL_BACKEND_RANKING_RUNTIME_FALLBACK_ENABLED` is explicitly enabled.
+- Added `backend/tests/test_ranking_snapshot_payload.py` with a regression test asserting PostgreSQL public reads do not invoke ranking snapshot storage initialization.
+
+Modified files:
+
+- `ai/tasks/done/TASK-215-optimize-weekly-monthly-ranking-read-path.md`
+- `backend/app/rcon_historical_leaderboards.py`
+- `backend/tests/test_ranking_snapshot_payload.py`
+
+Cause fixed:
+
+- `get_latest_ranking_snapshot()` initialized ranking snapshot storage before deciding the read path.
+- On the PostgreSQL public read path that pulled in setup or migration work through:
+  - `initialize_ranking_snapshot_storage()`
+  - `initialize_rcon_materialized_storage()`
+  - `initialize_postgres_rcon_storage()`
+- The fix separates public snapshot reads from generation and prevents those initializers from appearing in the default public weekly/monthly read path.
+
+Validation performed:
+
+- PASS: `python -m compileall backend\app\rcon_historical_leaderboards.py backend\tests\test_ranking_snapshot_payload.py`
+- PASS: `python -m unittest backend.tests.test_ranking_snapshot_payload`
+- PASS: direct timing probe through `build_global_ranking_payload(...)`
+- PASS: targeted `cProfile` probe confirms the profiled public read path now shows:
+  - `build_global_ranking_payload`
+  - `get_latest_ranking_snapshot`
+  - and does not show:
+    - `initialize_ranking_snapshot_storage`
+    - `initialize_rcon_materialized_storage`
+    - `initialize_postgres_rcon_storage`
+- INFO: HTTP probe against `http://127.0.0.1:8000` could not run because the local backend was not listening (`ConnectionRefusedError`).
+- INFO: local SQLite had no persisted weekly/monthly snapshots available:
+  - `ranking_snapshots = 0`
+  - `ranking_snapshot_items = 0`
+  - because of that, local payload validation exercised the fast missing-snapshot public path, not a ready snapshot payload with items.
+
+Before/after timing summary:
+
+- Before, per production evidence:
+  - `build_global_ranking_payload(timeframe="weekly", metric="kills", limit=20, server_id="all")` took `101.33 s`
+- After, local direct payload timing:
+  - weekly limit 20: `0.002 s`, `0.001 s`, `0.001 s`
+  - weekly limit 30: `0.001 s`, `0.001 s`, `0.001 s`
+  - monthly limit 20: `0.001 s`, `0.001 s`, `0.001 s`
+- Local response contract after the change in this environment:
+  - `snapshot_status = missing`
+  - `read_model = ranking-snapshot`
+  - `fallback_used = false`
+  - `items = 0`
+- Note: the `ready/items>0` contract could not be revalidated locally because no ranking snapshots existed in local storage.
+
+Scope confirmation:
+
+- No frontend file was touched.
+- No weapon asset was touched.
+- No SVG was touched.
+- No physical image was modified.
+- Elo/MMR was not reactivated.
+- Comunidad Hispana #03 was not reintroduced.
+- `ai/system-metrics.md` was not touched.
+- No push was made.
+
+## Change Budget
+
+- Prefer fewer than 5 modified files.
+- Prefer changes under 200 lines when feasible.
+- Split the work into follow-up tasks if limits are exceeded.

backend/app/rcon_historical_leaderboards.py

@@ -5,12 +5,18 @@ from __future__ import annotations
 import argparse
 import json
 import os
+import sqlite3
 from contextlib import closing
+from contextlib import contextmanager
 from datetime import date, datetime, timedelta, timezone
 from pathlib import Path
 from typing import Literal
 
-from .config import get_historical_weekly_fallback_min_matches, use_postgres_rcon_storage
+from .config import (
+    get_historical_weekly_fallback_min_matches,
+    get_storage_path,
+    use_postgres_rcon_storage,
+)
 from .historical_storage import ALL_SERVERS_SLUG
 from .rcon_admin_log_materialization import (
     MATCH_RESULT_SOURCE,
@@ -111,10 +117,10 @@ def initialize_ranking_snapshot_storage(*, db_path: Path | None = None) -> Path:
 
 
 def is_ranking_runtime_fallback_enabled() -> bool:
-    """Return whether `/api/ranking` may fall back to runtime reads when snapshot is missing."""
+    """Return whether `/api/ranking` may fall back to runtime reads in explicit internal mode."""
     normalized = os.getenv(
         "HLL_BACKEND_RANKING_RUNTIME_FALLBACK_ENABLED",
-        "true",
+        "false",
     ).strip().lower()
     return normalized in {"1", "true", "yes", "on"}
 
@@ -326,44 +332,36 @@ def get_latest_ranking_snapshot(
     normalized_metric = _normalize_metric(metric)
     normalized_limit = max(1, int(limit or 10))
 
-    resolved_path = initialize_ranking_snapshot_storage(db_path=db_path)
+    try:
-    connection_scope = _connect_scope(resolved_path, db_path=db_path)
+        with _open_ranking_snapshot_read_connection(db_path=db_path) as connection:
-    with connection_scope as connection:
+            snapshot = _find_latest_snapshot(
-        snapshot = _find_latest_snapshot(
+                connection=connection,
-            connection=connection,
+                timeframe=normalized_timeframe,
+                server_key=normalized_server_key,
+                metric=normalized_metric,
+            )
+            if snapshot is None:
+                return _build_missing_ranking_snapshot_result(
+                    timeframe=normalized_timeframe,
+                    server_key=normalized_server_key,
+                    metric=normalized_metric,
+                    limit=normalized_limit,
+                )
+
+            snapshot_limit = max(1, int(snapshot.get("limit_size") or normalized_limit))
+            item_count = int(snapshot.get("item_count") or 0)
+            effective_limit = max(0, min(normalized_limit, snapshot_limit, item_count))
+            items = _list_snapshot_items(
+                connection=connection,
+                snapshot_id=int(snapshot["id"]),
+                limit=effective_limit if effective_limit > 0 else None,
+            )
+    except (FileNotFoundError, sqlite3.OperationalError):
+        return _build_missing_ranking_snapshot_result(
             timeframe=normalized_timeframe,
             server_key=normalized_server_key,
             metric=normalized_metric,
-        )
+            limit=normalized_limit,
-        if snapshot is None:
-            return {
-                "snapshot_status": "missing",
-                "timeframe": normalized_timeframe,
-                "server_id": normalized_server_key,
-                "metric": normalized_metric,
-                "limit": normalized_limit,
-                "requested_limit": normalized_limit,
-                "effective_limit": 0,
-                "snapshot_limit": None,
-                "item_count": 0,
-                "generated_at": None,
-                "window_start": None,
-                "window_end": None,
-                "window_kind": None,
-                "window_label": None,
-                "source": "ranking-snapshot",
-                "freshness": "missing",
-                "source_matches_count": 0,
-                "items": [],
-            }
-
-        snapshot_limit = max(1, int(snapshot.get("limit_size") or normalized_limit))
-        item_count = int(snapshot.get("item_count") or 0)
-        effective_limit = max(0, min(normalized_limit, snapshot_limit, item_count))
-        items = _list_snapshot_items(
-            connection=connection,
-            snapshot_id=int(snapshot["id"]),
-            limit=effective_limit if effective_limit > 0 else None,
         )
 
     return {
@@ -388,6 +386,55 @@ def get_latest_ranking_snapshot(
     }
 
 
+@contextmanager
+def _open_ranking_snapshot_read_connection(*, db_path: Path | None = None):
+    if use_postgres_rcon_storage(explicit_sqlite_path=db_path):
+        from .postgres_rcon_storage import PostgresCompatConnection, connect_postgres
+
+        with connect_postgres() as connection:
+            yield PostgresCompatConnection(connection)
+        return
+
+    resolved_path = _resolve_ranking_snapshot_sqlite_path(db_path=db_path)
+    if not resolved_path.exists():
+        raise FileNotFoundError(resolved_path)
+    with closing(connect_sqlite_readonly(resolved_path)) as connection:
+        yield connection
+
+
+def _resolve_ranking_snapshot_sqlite_path(*, db_path: Path | None = None) -> Path:
+    return db_path if db_path is not None else get_storage_path()
+
+
+def _build_missing_ranking_snapshot_result(
+    *,
+    timeframe: str,
+    server_key: str,
+    metric: str,
+    limit: int,
+) -> dict[str, object]:
+    return {
+        "snapshot_status": "missing",
+        "timeframe": timeframe,
+        "server_id": server_key,
+        "metric": metric,
+        "limit": limit,
+        "requested_limit": limit,
+        "effective_limit": 0,
+        "snapshot_limit": None,
+        "item_count": 0,
+        "generated_at": None,
+        "window_start": None,
+        "window_end": None,
+        "window_kind": None,
+        "window_label": None,
+        "source": "ranking-snapshot",
+        "freshness": "missing",
+        "source_matches_count": 0,
+        "items": [],
+    }
+
+
 def build_rcon_materialized_leaderboard_snapshot_payload(
     *,
     server_id: str | None = None,

backend/tests/test_ranking_snapshot_payload.py

@@ -0,0 +1,33 @@
+import unittest
+from contextlib import nullcontext
+from unittest.mock import patch
+
+from app.rcon_historical_leaderboards import get_latest_ranking_snapshot
+
+
+class RankingSnapshotPayloadTests(unittest.TestCase):
+    def test_get_latest_ranking_snapshot_skips_storage_init_on_postgres_read(self):
+        with (
+            patch("app.rcon_historical_leaderboards.use_postgres_rcon_storage", return_value=True),
+            patch("app.rcon_historical_leaderboards.initialize_ranking_snapshot_storage") as init_mock,
+            patch(
+                "app.rcon_historical_leaderboards._open_ranking_snapshot_read_connection",
+                return_value=nullcontext(object()),
+            ),
+            patch("app.rcon_historical_leaderboards._find_latest_snapshot", return_value=None),
+        ):
+            result = get_latest_ranking_snapshot(
+                server_key="all",
+                timeframe="weekly",
+                metric="kills",
+                limit=20,
+            )
+
+        init_mock.assert_not_called()
+        self.assertEqual(result["snapshot_status"], "missing")
+        self.assertEqual(result["source"], "ranking-snapshot")
+        self.assertEqual(result["requested_limit"], 20)
+
+
+if __name__ == "__main__":
+    unittest.main()