diff --git a/ai/tasks/done/TASK-215-optimize-weekly-monthly-ranking-read-path.md b/ai/tasks/done/TASK-215-optimize-weekly-monthly-ranking-read-path.md new file mode 100644 index 0000000..fe79050 --- /dev/null +++ b/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\.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. diff --git a/backend/app/rcon_historical_leaderboards.py b/backend/app/rcon_historical_leaderboards.py index 7e547c9..41633cd 100644 --- a/backend/app/rcon_historical_leaderboards.py +++ b/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) - connection_scope = _connect_scope(resolved_path, db_path=db_path) - with connection_scope as connection: - snapshot = _find_latest_snapshot( - connection=connection, + try: + with _open_ranking_snapshot_read_connection(db_path=db_path) as connection: + snapshot = _find_latest_snapshot( + 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, - ) - 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, + limit=normalized_limit, ) 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, diff --git a/backend/tests/test_ranking_snapshot_payload.py b/backend/tests/test_ranking_snapshot_payload.py new file mode 100644 index 0000000..5a9d7fe --- /dev/null +++ b/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()