From 279fcb04a66d4fae54a3bd13b5cf6702bec90e50 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 09:44:48 +0200 Subject: [PATCH] Fix game-mode config: load env recommender settings, not stale defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Game Mode, GameShell mounts and immediately drives loadGameStudy(), which built the POST /api/config payload from buildConfigRequest() — the live useSettings state. But useSettings only loads the active environment config (e.g. min_redispatch=2) via an async mount effect that hasn't resolved yet, so the request fell back to the hardcoded defaults (min_redispatch=0, n_prioritized_actions=10, allowed_action_types=[]) for everything except the network/action/layout paths the study overrides. Two symptoms: - The recommender ran on default settings instead of the loaded environment's config ("config not set on the proper environment"). - min_redispatch=0 reached the backend, so the expert recommender never forced redispatch into the prioritized feed (redispatch is only guaranteed via the MIN_REDISPATCH minimum-enforcement phase; the fill phase is crowded out by disco/coupling candidates). Redispatch actions were scored in the Manual Selection table but absent from Suggested Actions. Fix: source loadGameStudy's config from the freshly-loaded active user config via configRequestFromUserConfig(), mirroring the stale-closure guard already in handleLoadConfig. The study still overrides only the network/action/layout paths + contingency. Add App.gameMode.test.tsx: a deterministic regression test that holds the config fetch open to reproduce the race, then asserts the /api/config payload carries min_redispatch=2 (not the default 0) while the study path override still wins. Co-Authored-By: Claude Opus 4.8 --- frontend/src/App.gameMode.test.tsx | 154 +++++++++++++++++++++++++++++ frontend/src/App.tsx | 19 +++- 2 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 frontend/src/App.gameMode.test.tsx diff --git a/frontend/src/App.gameMode.test.tsx b/frontend/src/App.gameMode.test.tsx new file mode 100644 index 00000000..30a693b8 --- /dev/null +++ b/frontend/src/App.gameMode.test.tsx @@ -0,0 +1,154 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, act, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom/vitest'; +import App from './App'; +import { gameBridge } from './game/gameBridge'; +import type { GameStudy } from './game/types'; +import type { UserConfig } from './api'; + +// Keep the App tree light — these presentational pieces are irrelevant to the +// config-propagation path under test (mirrors App.configUpload.test.tsx). +vi.mock('./components/VisualizationPanel', () => ({ default: () =>
})); +vi.mock('./components/ActionFeed', () => ({ default: () =>
})); +vi.mock('./components/OverloadPanel', () => ({ default: () =>
})); +vi.mock('./hooks/usePanZoom', () => ({ usePanZoom: () => ({ viewBox: null, setViewBox: vi.fn() }) })); +vi.mock('./utils/svgUtils', () => ({ + processSvg: (svg: string) => ({ svg, viewBox: { x: 0, y: 0, w: 100, h: 100 } }), + buildMetadataIndex: () => null, + applyOverloadedHighlights: vi.fn(), + applyDeltaVisuals: vi.fn(), + applyActionTargetHighlights: vi.fn(), + applyContingencyHighlight: vi.fn(), + getIdMap: () => new Map(), + invalidateIdMapCache: vi.fn(), + isCouplingAction: vi.fn(() => false), + attachVlInteractions: vi.fn(() => () => {}), +})); + +// The active persisted environment config (what `config_path.txt` points at — +// e.g. config_pypsa_eur_eur220_225_380_400.json). Crucially it carries +// `min_redispatch: 2` and a NETWORK PATH that differs from the study's, so the +// two assertions below are independent: the recommender minima must come from +// THIS config, while the network path must come from the study override. +const ENV_CONFIG: UserConfig = { + network_path: 'data/active_env/network.xiidm', + action_file_path: 'data/active_env/actions.json', + layout_path: 'data/active_env/grid_layout.json', + output_folder_path: '', + lines_monitoring_path: '', + min_line_reconnections: 2, + min_close_coupling: 3, + min_open_coupling: 2, + min_line_disconnections: 3, + min_pst: 1, + min_load_shedding: 2, + min_renewable_curtailment_actions: 2, + min_redispatch: 2, + allowed_action_types: [], + n_prioritized_actions: 15, + monitoring_factor: 0.95, + pre_existing_overload_threshold: 0.02, + ignore_reconnections: false, + pypowsybl_fast_mode: true, + model: 'expert', + compute_overflow_graph: true, +}; + +const STUDY: GameStudy = { + id: 'eu-pyrenees', + label: 'Pyrenees (France / Spain) 225 kV — LANNEL61PRAGN', + networkPath: 'data/pypsa_eur_eur220_225_380_400/network.xiidm', + actionFilePath: 'data/pypsa_eur_eur220_225_380_400/actions.json', + layoutPath: 'data/pypsa_eur_eur220_225_380_400/grid_layout.json', + // Empty so loadGameStudy skips arming a contingency (no N-1 fetch needed — + // this test is only about the /api/config payload). + contingencyElementId: '', +}; + +// `getUserConfig` is deferred per-test so we can hold the config fetch open and +// reproduce the real race: GameShell mounts and fires the study loader +// BEFORE useSettings' async getUserConfig() effect has applied to state. +let resolveUserConfig: (cfg: UserConfig) => void; + +const mockApi = vi.hoisted(() => ({ + getUserConfig: vi.fn(), + getConfigFilePath: vi.fn().mockResolvedValue('/active/config.json'), + saveUserConfig: vi.fn().mockResolvedValue({}), + getModels: vi.fn().mockResolvedValue({ models: [] }), + setRecommenderModel: vi.fn().mockResolvedValue({}), + updateConfig: vi.fn().mockResolvedValue({ monitored_lines_count: 0, total_lines_count: 0 }), + getBranches: vi.fn().mockResolvedValue({ branches: [], name_map: {} }), + getVoltageLevels: vi.fn().mockResolvedValue({ voltage_levels: [], name_map: {} }), + getNominalVoltages: vi.fn().mockResolvedValue({ mapping: {}, unique_kv: [] }), + getVoltageLevelSubstations: vi.fn().mockResolvedValue({ mapping: {} }), + getNetworkDiagram: vi.fn().mockResolvedValue({ svg: '', metadata: null }), +})); +vi.mock('./api', () => ({ api: mockApi })); + +afterEach(() => { + cleanup(); + window.history.replaceState({}, '', '/'); +}); + +describe('Game Mode config propagation', () => { + beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + // Launch in game mode so App registers its study loader on the bridge. + window.history.replaceState({}, '', '/?game=1'); + // Deferred — left pending until we choose to release it. + mockApi.getUserConfig.mockReturnValue( + new Promise((res) => { resolveUserConfig = res; }), + ); + }); + + it('loadGameStudy sends the persisted env config minima (min_redispatch=2), not stale useSettings defaults, while the study path override still wins', async () => { + render(); + + // useSettings' mount effect is now blocked on the deferred getUserConfig, + // so the recommender settings in component state are at their hardcoded + // defaults (minRedispatch = 0, nPrioritizedActions = 10). App's game-mode + // effect has synchronously registered the loader on the bridge. + + // Drive the study load WHILE the config fetch is still in flight. The + // pre-fix code read `buildConfigRequest()` (stale state) here and posted + // min_redispatch=0; the fix awaits the persisted config instead. + let loadPromise!: Promise; + await act(async () => { + loadPromise = gameBridge.loadStudy(STUDY); + // let loadGameStudy run up to its `await api.getUserConfig()` + await Promise.resolve(); + }); + + // Suspended on the deferred config — nothing posted yet. + expect(mockApi.updateConfig).not.toHaveBeenCalled(); + + // Release the active environment config (the one carrying min_redispatch=2). + await act(async () => { + resolveUserConfig(ENV_CONFIG); + await loadPromise; + }); + + expect(mockApi.updateConfig).toHaveBeenCalledTimes(1); + const sent = mockApi.updateConfig.mock.calls[0][0]; + + // Recommender settings come from the persisted env config — the regression: + // these must NOT be the useSettings defaults (0 / [] / 10). + expect(sent).toMatchObject({ + min_redispatch: 2, + min_load_shedding: 2, + min_renewable_curtailment_actions: 2, + allowed_action_types: [], + n_prioritized_actions: 15, + model: 'expert', + }); + + // …but the network / action / layout PATHS are still overridden by the + // study (they differ from the env config's own paths on purpose). + expect(sent).toMatchObject({ + network_path: STUDY.networkPath, + action_file_path: STUDY.actionFilePath, + layout_path: STUDY.layoutPath, + }); + }); +}); diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 41b57d5d..78b2451e 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1396,8 +1396,22 @@ function App() { if (study.layoutPath !== undefined) setLayoutPath(study.layoutPath); if (study.linesMonitoringPath !== undefined) setLinesMonitoringPath(study.linesMonitoringPath); try { + // Recommender settings (per-type minima, model, monitoring factor, …) + // must come from the active persisted environment config, NOT the + // in-memory `useSettings` state. GameShell mounts and fires this + // loader before useSettings' async `getUserConfig()` effect has resolved, + // so `buildConfigRequest()` here would replay the hardcoded defaults + // (min_redispatch=0, n_prioritized_actions=10, allowed_action_types=[], …) + // and the environment's real config (e.g. min_redispatch=2) would be + // silently dropped — the same stale-closure trap `handleLoadConfig` + // guards against. The study only dictates the network / action / layout + // PATHS + contingency; everything else comes from the env config. + const freshCfg = await api.getUserConfig().catch(() => null); + const baseConfig = freshCfg + ? configRequestFromUserConfig(freshCfg) + : buildConfigRequest(); const configRequest = { - ...buildConfigRequest(), + ...baseConfig, network_path: study.networkPath, action_file_path: study.actionFilePath, layout_path: study.layoutPath ?? '', @@ -1445,7 +1459,8 @@ function App() { setConfigLoading(false); } }, [ - buildConfigRequest, applyConfigResponse, resetAllState, diagrams, + buildConfigRequest, configRequestFromUserConfig, applyConfigResponse, + resetAllState, diagrams, setNetworkPath, setActionPath, setLayoutPath, setLinesMonitoringPath, setBranches, setVoltageLevels, setNameMap, setPendingContingency, setVlToSubstation, setSelectedContingency, setConfigLoading, setError,