diff --git a/ai/tasks/done/TASK-195-fix-ranking-snapshot-generator-postgres-default.md b/ai/tasks/done/TASK-195-fix-ranking-snapshot-generator-postgres-default.md new file mode 100644 index 0000000..87690d2 --- /dev/null +++ b/ai/tasks/done/TASK-195-fix-ranking-snapshot-generator-postgres-default.md @@ -0,0 +1,134 @@ +--- +id: TASK-195-fix-ranking-snapshot-generator-postgres-default +title: Fix ranking snapshot generator PostgreSQL default +status: done +type: backend +team: Backend Senior +supporting_teams: + - Arquitecto de Base de Datos + - Arquitecto Python +roadmap_item: foundation +priority: high +--- + +# TASK-195 - Fix ranking snapshot generator PostgreSQL default + +## Goal + +Correct the weekly/monthly ranking snapshot CLI so production generation uses PostgreSQL by default, matching the operational `/api/ranking` read path. + +## Context + +`TASK-194` added a manual weekly/monthly snapshot generator, but the CLI currently passes `db_path=get_storage_path()` into `generate_ranking_snapshot(...)`. In this repository, `use_postgres_rcon_storage(...)` only enables PostgreSQL when `explicit_sqlite_path is None` and `HLL_BACKEND_DATABASE_URL` exists, so the current CLI forces SQLite while `/api/ranking` reads PostgreSQL. + +This causes operational commands to generate `ready` snapshots with `item_count=0` and `source_matches_count=0` in SQLite even when runtime fallback over PostgreSQL returns ranking players. + +Preserve the current product identity: Spanish-speaking HLL Vietnam community, military/Vietnam/tactical/sober visual direction and controlled repository evolution. + +## Steps + +1. Read the listed files first. +2. Change the ranking snapshot CLI to use PostgreSQL by default in operational mode. +3. Keep SQLite available only as an explicit local-development override if needed. +4. Validate the CLI default path, repository scripts and documentation. +5. Document root cause, fix and production validation steps. + +## Files to Read First + +- `AGENTS.md` +- `ai/repo-context.md` +- `ai/architecture-index.md` +- `backend/app/config.py` +- `backend/app/rcon_historical_leaderboards.py` +- `backend/app/postgres_rcon_storage.py` +- `scripts/run-stats-validation.ps1` +- `docs/ranking-snapshot-read-model-plan.md` +- `ai/tasks/done/TASK-194-add-weekly-monthly-ranking-snapshot-generator.md` + +## Expected Files to Modify + +- `backend/app/rcon_historical_leaderboards.py` +- `scripts/run-stats-validation.ps1` +- `docs/ranking-snapshot-read-model-plan.md` +- `ai/tasks/done/TASK-195-fix-ranking-snapshot-generator-postgres-default.md` + +## Constraints + +- Keep the change minimal. +- Do not modify frontend, assets or design. +- Do not change public endpoint behavior beyond the intended generator hotfix. +- Keep annual ranking behavior unchanged. +- Do not reactivate Elo/MMR. +- Do not reintroduce Comunidad Hispana #03. +- Keep PostgreSQL as the operational default and SQLite only as an explicit local-development mode. + +## Validation + +Before completing the task ensure: + +- `powershell -ExecutionPolicy Bypass -File scripts/run-stats-validation.ps1` +- `powershell -ExecutionPolicy Bypass -File scripts/run-integration-tests.ps1` +- local import or parser validation proves the CLI no longer passes `get_storage_path()` by default +- local validation proves `generate_ranking_snapshot(..., db_path=None)` uses PostgreSQL when `HLL_BACKEND_DATABASE_URL` is configured +- `git diff --name-only` matches the expected scope + +## Outcome + +Root cause: + +- the manual CLI in `backend/app/rcon_historical_leaderboards.py` called `generate_ranking_snapshot(..., db_path=get_storage_path())` +- that forced `explicit_sqlite_path != None` +- `use_postgres_rcon_storage(...)` therefore disabled PostgreSQL even when `HLL_BACKEND_DATABASE_URL` was configured +- operational `/api/ranking` reads PostgreSQL, so the generator and the public ranking endpoint diverged onto different storage backends + +Applied change: + +- `backend/app/rcon_historical_leaderboards.py` + - removed the default CLI path that forced SQLite + - changed the operational default to `db_path=None` + - added explicit `--sqlite-path ` override for local development only +- `scripts/run-stats-validation.ps1` + - now validates that the CLI default passes `db_path=None` + - now validates that PostgreSQL selection activates when `HLL_BACKEND_DATABASE_URL` is configured and no explicit SQLite path is provided +- `docs/ranking-snapshot-read-model-plan.md` + - now documents PostgreSQL as the operational default + - now documents `--sqlite-path` as an explicit local override only + +Previous vs new behavior: + +- before: + - `python -m app.rcon_historical_leaderboards generate-ranking-snapshot ...` + - forced SQLite by default + - could generate empty snapshots operationally while `/api/ranking` fallback over PostgreSQL still returned players +- now: + - the same command uses PostgreSQL by default when `HLL_BACKEND_DATABASE_URL` is configured + - SQLite is used only when the operator passes `--sqlite-path` + +Validation executed: + +- `powershell -ExecutionPolicy Bypass -File scripts/run-stats-validation.ps1` +- `powershell -ExecutionPolicy Bypass -File scripts/run-integration-tests.ps1` + +Validation notes: + +- live backend HTTP at `http://127.0.0.1:8000` was not available in this environment +- route and CLI validation completed through local Python imports and repository validation scripts +- no local PostgreSQL instance was required because the validation proved backend selection by inspection and monkeypatched connection-path checks + +Final recommended Docker command: + +- `docker compose exec backend python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20` + +How to validate in production: + +1. run the Docker command above in the production backend container +2. query PostgreSQL `ranking_snapshots` for the selected `(timeframe, server_id, metric, window_start, window_end)` +3. confirm `item_count > 0` and `source_matches_count > 0` for covered windows +4. call `/api/ranking?timeframe=weekly&server_id=all&metric=kills&limit=20` +5. confirm `snapshot_status=ready` and `fallback_used=false` + +## 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 694c6d4..d8a6eb4 100644 --- a/backend/app/rcon_historical_leaderboards.py +++ b/backend/app/rcon_historical_leaderboards.py @@ -10,8 +10,7 @@ from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Literal -from .config import get_storage_path, use_postgres_rcon_storage -from .config import get_historical_weekly_fallback_min_matches +from .config import get_historical_weekly_fallback_min_matches, use_postgres_rcon_storage from .historical_storage import ALL_SERVERS_SLUG from .rcon_admin_log_materialization import ( MATCH_RESULT_SOURCE, @@ -1256,6 +1255,12 @@ def _main(argv: list[str] | None = None) -> int: choices=SNAPSHOT_GENERATOR_METRICS, ) generate_parser.add_argument("--limit", type=int, default=20) + generate_parser.add_argument( + "--sqlite-path", + type=Path, + default=None, + help="explicit local SQLite override; default operational mode uses PostgreSQL when configured", + ) generate_parser.add_argument( "--no-replace-existing", action="store_false", @@ -1272,7 +1277,7 @@ def _main(argv: list[str] | None = None) -> int: metric=args.metric, limit=args.limit, replace_existing=args.replace_existing, - db_path=get_storage_path(), + db_path=args.sqlite_path, ) print(json.dumps({"status": "ok", "data": payload}, ensure_ascii=True, indent=2)) return 0 diff --git a/docs/ranking-snapshot-read-model-plan.md b/docs/ranking-snapshot-read-model-plan.md index 076bc09..66036b8 100644 --- a/docs/ranking-snapshot-read-model-plan.md +++ b/docs/ranking-snapshot-read-model-plan.md @@ -253,12 +253,24 @@ Manual generator entrypoint: python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20 ``` +Operational default: +- when `HLL_BACKEND_DATABASE_URL` is configured, the CLI uses PostgreSQL by default +- SQLite is no longer the default operational target for snapshot generation +- local SQLite generation remains available only through an explicit override such as: + +```bash +python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20 --sqlite-path backend/data/hll_vietnam_dev.sqlite3 +``` + Docker form: ```bash docker compose exec backend python -m app.rcon_historical_leaderboards generate-ranking-snapshot --timeframe weekly --server-key all --metric kills --limit 20 ``` +Operational expectation: +- the recommended Docker command should generate weekly/monthly snapshots in PostgreSQL, matching the `/api/ranking` production read path + Supported manual parameters: - `timeframe`: `weekly`, `monthly` - `server-key`: `all`, `all-servers`, `comunidad-hispana-01`, `comunidad-hispana-02` diff --git a/scripts/run-stats-validation.ps1 b/scripts/run-stats-validation.ps1 index cf6945f..6882dc8 100644 --- a/scripts/run-stats-validation.ps1 +++ b/scripts/run-stats-validation.ps1 @@ -117,12 +117,15 @@ import json import os import sqlite3 import sys +from contextlib import contextmanager, redirect_stdout from datetime import datetime, timezone +from io import StringIO from pathlib import Path sys.path.insert(0, "backend") from app.routes import resolve_get_payload +from app.config import use_postgres_rcon_storage import app.postgres_rcon_storage as postgres_rcon_storage import app.rcon_historical_leaderboards as ranking_leaderboards @@ -284,6 +287,90 @@ def validate_postgres_ranking_snapshot_schema_path(): ) +def validate_ranking_snapshot_cli_defaults(): + original_generate_ranking_snapshot = ranking_leaderboards.generate_ranking_snapshot + captured = {} + + def fake_generate_ranking_snapshot(**kwargs): + captured.update(kwargs) + return {"status": "ok", "snapshot": None, "items": []} + + ranking_leaderboards.generate_ranking_snapshot = fake_generate_ranking_snapshot + + try: + stdout_buffer = StringIO() + with redirect_stdout(stdout_buffer): + exit_code = ranking_leaderboards._main([ + "generate-ranking-snapshot", + "--timeframe", "weekly", + "--server-key", "all", + "--metric", "kills", + "--limit", "20", + ]) + require(exit_code == 0, "Ranking snapshot CLI should exit 0 for a valid command.") + require( + captured.get("db_path") is None, + "Ranking snapshot CLI should use PostgreSQL-compatible db_path=None by default.", + ) + + captured.clear() + with redirect_stdout(stdout_buffer): + exit_code = ranking_leaderboards._main([ + "generate-ranking-snapshot", + "--timeframe", "weekly", + "--server-key", "all", + "--metric", "kills", + "--limit", "20", + "--sqlite-path", "backend/data/hll_vietnam_dev.sqlite3", + ]) + require(exit_code == 0, "Ranking snapshot CLI with --sqlite-path should exit 0.") + require( + captured.get("db_path") == Path("backend/data/hll_vietnam_dev.sqlite3"), + "Ranking snapshot CLI should pass the explicit --sqlite-path override through to generate_ranking_snapshot.", + ) + finally: + ranking_leaderboards.generate_ranking_snapshot = original_generate_ranking_snapshot + + +def validate_ranking_snapshot_postgres_selection(): + original_database_url = os.environ.get("HLL_BACKEND_DATABASE_URL") + original_connect_postgres_compat = postgres_rcon_storage.connect_postgres_compat + calls = {"postgres_connect": 0} + + @contextmanager + def fake_connect_postgres_compat(): + calls["postgres_connect"] += 1 + yield object() + + os.environ["HLL_BACKEND_DATABASE_URL"] = "postgresql://validation-user:validation-pass@127.0.0.1:5432/hll_validation" + postgres_rcon_storage.connect_postgres_compat = fake_connect_postgres_compat + + try: + require( + use_postgres_rcon_storage(explicit_sqlite_path=None) is True, + "PostgreSQL storage should be selected when DATABASE_URL is configured and no explicit SQLite path is provided.", + ) + require( + use_postgres_rcon_storage(explicit_sqlite_path=Path("backend/data/hll_vietnam_dev.sqlite3")) is False, + "Explicit SQLite paths should still disable PostgreSQL storage selection.", + ) + with ranking_leaderboards._connect_write_scope( + Path("backend/data/hll_vietnam_dev.sqlite3"), + db_path=None, + ) as _connection: + pass + require( + calls["postgres_connect"] == 1, + "Ranking snapshot write scope should use PostgreSQL when db_path=None and DATABASE_URL is configured.", + ) + finally: + if original_database_url is None: + os.environ.pop("HLL_BACKEND_DATABASE_URL", None) + else: + os.environ["HLL_BACKEND_DATABASE_URL"] = original_database_url + postgres_rcon_storage.connect_postgres_compat = original_connect_postgres_compat + + def cleanup_snapshot_fixture(db_path): connection = sqlite3.connect(db_path) with connection: @@ -325,6 +412,8 @@ require(health_status == 200, "Route resolver /health should return 200.") require(health_payload.get("status") == "ok", "/health payload should be ok.") validate_postgres_ranking_snapshot_schema_path() +validate_ranking_snapshot_cli_defaults() +validate_ranking_snapshot_postgres_selection() kd_metric_sql, _, _ = ranking_leaderboards._resolve_metric_sql("kd_ratio") require( @@ -694,8 +783,10 @@ print(json.dumps({ "stats-player-profile", "stats-annual-ranking", "global-ranking", - "postgres-ranking-derived-metric-sql", + "postgres-ranking-derived-metric-sql", "postgres-ranking-schema-path", + "ranking-snapshot-cli-postgres-default", + "ranking-snapshot-postgres-selection", "ranking-snapshot-generator", "ranking-snapshot-ready", "ranking-snapshot-missing",