From d225e243ad2a2365c402ee284ad4ad424615281c Mon Sep 17 00:00:00 2001 From: devRaGonSa Date: Thu, 11 Jun 2026 10:34:51 +0200 Subject: [PATCH] Fix public snapshot runner initialize storage argument --- ...shot-runner-initialize-storage-argument.md | 70 ++++++++++++++++ backend/app/rcon_historical_leaderboards.py | 14 +++- .../tests/test_historical_snapshot_refresh.py | 82 +++++++++++++++++++ docs/public-snapshot-refresh-schedule.md | 1 + 4 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 ai/tasks/done/TASK-245-fix-public-snapshot-runner-initialize-storage-argument.md diff --git a/ai/tasks/done/TASK-245-fix-public-snapshot-runner-initialize-storage-argument.md b/ai/tasks/done/TASK-245-fix-public-snapshot-runner-initialize-storage-argument.md new file mode 100644 index 0000000..c77c023 --- /dev/null +++ b/ai/tasks/done/TASK-245-fix-public-snapshot-runner-initialize-storage-argument.md @@ -0,0 +1,70 @@ +--- +id: TASK-245 +title: Fix public snapshot runner initialize storage argument +status: done +type: backend +team: Backend Senior +supporting_teams: + - Arquitecto Python +roadmap_item: foundation +priority: high +--- + +# TASK-245 - Fix public snapshot runner initialize storage argument + +## Goal + +Fix the deployed manual public jobs that failed with `initialize_rcon_materialized_storage() got an unexpected keyword argument 'ensure_storage'`. + +## Context + +TASK-243 added `ensure_storage` gating in the ranking and historical snapshot paths, but one wrapper incorrectly propagated that keyword into `initialize_rcon_materialized_storage()`, whose signature only accepts `db_path`. + +The failure was visible when running: + +- `python -m app.historical_runner --public-job ranking-weekly` +- `python -m app.historical_runner --public-job ranking-monthly` +- `python -m app.historical_runner --public-job historical-weekly` +- `python -m app.historical_runner --public-job historical-monthly` + +## Steps + +1. Audited every `ensure_storage=` path around public snapshot generation. +2. Confirmed that `initialize_rcon_materialized_storage()` does not accept that keyword. +3. Moved the gating logic to a local path resolver in `rcon_historical_leaderboards`. +4. Kept `connect_postgres_compat(initialize=False)` for follow-up read/write connections after the one-time initialization point. +5. Added regression tests for both enabled and disabled initialization flows. + +## Files to Read First + +- `AGENTS.md` +- `ai/architecture-index.md` +- `ai/repo-context.md` +- `backend/app/historical_runner.py` +- `backend/app/rcon_historical_leaderboards.py` +- `backend/app/rcon_admin_log_materialization.py` + +## Expected Files to Modify + +- `backend/app/rcon_historical_leaderboards.py` +- `backend/tests/test_historical_snapshot_refresh.py` +- `docs/public-snapshot-refresh-schedule.md` + +## Constraints + +- Keep public GET endpoints snapshot-only. +- Do not touch frontend, assets or server configuration. +- Preserve the TASK-243 fixes for JSON serialization, duplicate ranking items and monthly MVP V2 fallback. + +## Validation + +- `python -m compileall backend/app` +- `cd backend; python -m unittest tests.test_historical_snapshot_refresh` +- `cd backend; python -m unittest tests.test_current_match_payload tests.test_rcon_admin_log_storage tests.test_historical_snapshot_refresh` + +## Outcome + +- Manual public jobs no longer pass an invalid keyword to `initialize_rcon_materialized_storage()`. +- `ensure_storage=True` still initializes materialized storage once through the wrapper layer. +- `ensure_storage=False` now skips the base initializer cleanly and relies on the existing non-DDL connection path. +- Regression coverage now asserts both enabled and disabled initialization flows. diff --git a/backend/app/rcon_historical_leaderboards.py b/backend/app/rcon_historical_leaderboards.py index 54d0309..b61a1e5 100644 --- a/backend/app/rcon_historical_leaderboards.py +++ b/backend/app/rcon_historical_leaderboards.py @@ -107,7 +107,7 @@ def initialize_ranking_snapshot_storage( ensure_storage: bool = True, ) -> Path: """Create ranking snapshot tables used by weekly/monthly public ranking reads.""" - resolved_path = initialize_rcon_materialized_storage( + resolved_path = _resolve_materialized_storage_path( db_path=db_path, ensure_storage=ensure_storage, ) @@ -430,6 +430,16 @@ def _resolve_ranking_snapshot_sqlite_path(*, db_path: Path | None = None) -> Pat return db_path if db_path is not None else get_storage_path() +def _resolve_materialized_storage_path( + *, + db_path: Path | None = None, + ensure_storage: bool = True, +) -> Path: + if ensure_storage: + return initialize_rcon_materialized_storage(db_path=db_path) + return _resolve_ranking_snapshot_sqlite_path(db_path=db_path) + + def _build_missing_ranking_snapshot_result( *, timeframe: str, @@ -566,7 +576,7 @@ def list_rcon_materialized_leaderboard( normalized_limit = max(1, int(limit or 10)) anchor = _as_utc(now or datetime.now(timezone.utc)) - resolved_path = initialize_rcon_materialized_storage( + resolved_path = _resolve_materialized_storage_path( db_path=db_path, ensure_storage=ensure_storage, ) diff --git a/backend/tests/test_historical_snapshot_refresh.py b/backend/tests/test_historical_snapshot_refresh.py index e8a03fc..cb500dd 100644 --- a/backend/tests/test_historical_snapshot_refresh.py +++ b/backend/tests/test_historical_snapshot_refresh.py @@ -50,6 +50,8 @@ from app.rcon_historical_read_model import ( _calculate_duration_seconds, ) from app.rcon_historical_leaderboards import _dedupe_snapshot_rows +from app.rcon_historical_leaderboards import initialize_ranking_snapshot_storage +from app.rcon_historical_leaderboards import list_rcon_materialized_leaderboard class HistoricalSnapshotRefreshTests(unittest.TestCase): @@ -306,6 +308,86 @@ class HistoricalSnapshotRefreshTests(unittest.TestCase): self.assertEqual(payload["event_coverage"]["reason"], "player-event-raw-ledger-missing") self.assertEqual(payload["items"], []) + def test_initialize_ranking_snapshot_storage_uses_materialized_initializer_without_invalid_keyword(self) -> None: + db_path = Path("dummy.sqlite3") + with ( + patch( + "app.rcon_historical_leaderboards.initialize_rcon_materialized_storage", + return_value=db_path, + ) as materialized_initializer, + patch("app.rcon_historical_leaderboards.use_postgres_rcon_storage", return_value=False), + patch( + "app.rcon_historical_leaderboards.connect_sqlite_writer", + return_value=sqlite3.connect(":memory:"), + ), + ): + resolved_path = initialize_ranking_snapshot_storage(db_path=db_path, ensure_storage=True) + + materialized_initializer.assert_called_once_with(db_path=db_path) + self.assertEqual(resolved_path, db_path) + + def test_initialize_ranking_snapshot_storage_skips_materialized_initializer_when_disabled(self) -> None: + db_path = Path("dummy.sqlite3") + with ( + patch("app.rcon_historical_leaderboards.initialize_rcon_materialized_storage") as materialized_initializer, + patch("app.rcon_historical_leaderboards.use_postgres_rcon_storage", return_value=False), + patch( + "app.rcon_historical_leaderboards.connect_sqlite_writer", + return_value=sqlite3.connect(":memory:"), + ), + ): + resolved_path = initialize_ranking_snapshot_storage(db_path=db_path, ensure_storage=False) + + materialized_initializer.assert_not_called() + self.assertEqual(resolved_path, db_path) + + def test_list_rcon_materialized_leaderboard_skips_materialized_initializer_when_disabled(self) -> None: + db_path = Path("dummy.sqlite3") + window = { + "start": datetime(2026, 6, 9, tzinfo=timezone.utc), + "end": datetime(2026, 6, 10, tzinfo=timezone.utc), + "days": 1, + "kind": "current-week", + "label": "Semana actual", + "selection_reason": "test", + "current_week_start": datetime(2026, 6, 9, tzinfo=timezone.utc), + "current_week_closed_matches": 1, + "previous_week_closed_matches": 2, + "current_month_start": datetime(2026, 6, 1, tzinfo=timezone.utc), + "selected_month_start": datetime(2026, 6, 1, tzinfo=timezone.utc), + "selected_month_end": datetime(2026, 6, 10, tzinfo=timezone.utc), + "current_month_closed_matches": 3, + "previous_month_closed_matches": 4, + "sufficient_sample": {"minimum_closed_matches": 1}, + } + fake_connection = object() + with ( + patch("app.rcon_historical_leaderboards.initialize_rcon_materialized_storage") as materialized_initializer, + patch( + "app.rcon_historical_leaderboards._connect_scope", + return_value=nullcontext(fake_connection), + ) as connect_scope, + patch("app.rcon_historical_leaderboards.select_leaderboard_window", return_value=window), + patch("app.rcon_historical_leaderboards._fetch_leaderboard_rows", return_value=[]), + patch( + "app.rcon_historical_leaderboards._fetch_source_range", + return_value=(None, None), + ), + ): + payload = list_rcon_materialized_leaderboard( + server_key="all-servers", + timeframe="weekly", + metric="kills", + limit=10, + ensure_storage=False, + db_path=db_path, + now=datetime(2026, 6, 10, tzinfo=timezone.utc), + ) + + materialized_initializer.assert_not_called() + connect_scope.assert_called_once_with(db_path, db_path=db_path, initialize=False) + self.assertEqual(payload["items"], []) + def test_historical_leaderboard_snapshot_does_not_runtime_enrich_public_request(self) -> None: snapshot = { "generated_at": "2026-06-10T04:00:00Z", diff --git a/docs/public-snapshot-refresh-schedule.md b/docs/public-snapshot-refresh-schedule.md index ca45ef8..7034621 100644 --- a/docs/public-snapshot-refresh-schedule.md +++ b/docs/public-snapshot-refresh-schedule.md @@ -102,6 +102,7 @@ Manual public jobs executed with `python -m app.historical_runner --public-job . - ranking snapshot rows are deduplicated by `player_id` before inserting `ranking_snapshot_items` - PostgreSQL snapshot storage initialization runs once at the start of the heavy job, then substeps reuse non-DDL connections - missing `player_event_raw_ledger` no longer aborts `historical-monthly`; the monthly MVP V2 slice degrades to an empty payload with `event_coverage.ready = false` +- materialized storage initialization gating stays in the leaderboard wrapper layer; `initialize_rcon_materialized_storage()` is called without extra keywords only when initialization is actually required This keeps manual validation commands useful without hiding partial failures inside the job payload.