From 1b1102a5f87407b195c6b19d8a23787d23103ed2 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Wed, 8 Jul 2026 20:06:41 +0000 Subject: [PATCH 01/24] feat: day-one quick-win bundle (QW3/4/7/9) from the 2026-07 review Lands the cheap halves of the two still-open headline issues (#4 invisible failures, #5 trust model) plus dead-code cleanup: - QW4: surface useAnalysis.error through StatusToasts (error || analysis.error, cleared on contingency-clear) so an analysis failure is no longer a silent dead spinner; the last silent SLD-preview catch now setError's too. Guarded by a new App-integration test. - QW3: CORS default "*" -> loopback dev origins (localhost/127.0.0.1 on :5173/:4173); the wildcard is now explicit opt-in via CORS_ALLOWED_ORIGINS="*", closing the drive-by cross-origin read of a localhost backend. - QW7: _safe_session_dir rejects "..", path separators and absolute session names before any filesystem write (resolve()+relative_to() backstop mirroring the /results/pdf guard); applied to save-session + load-session. Guarded by TestSessionPathTraversal. - QW9: delete inspect_action.py, the dead expert_backend/requirements.txt stub, and the unused react-zoom-pan-pinch dependency (+ lockfile and doc follow-through). The tracked Overflow_Graph/*.html is deliberately kept: it is a skip-guarded regression fixture and untracking it would silently drop that CI coverage. Docs (CLAUDE.md trees, README, .env.example) reconciled with the CORS default change, the removed files, and the react-zoom-pan-pinch removal (the "rendered inside react-zoom-pan-pinch" claims corrected to the actual usePanZoom viewBox mechanism); review tracker updated. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- .env.example | 8 ++- .github/workflows/deploy-huggingface.yml | 2 +- CLAUDE.md | 13 ++-- README.md | 1 - docs/architecture/2026-07-full-repo-review.md | 24 ++++--- expert_backend/CLAUDE.md | 13 ++-- expert_backend/main.py | 49 +++++++++++-- expert_backend/requirements.txt | 5 -- expert_backend/tests/test_api_endpoints.py | 70 +++++++++++++++++++ frontend/CLAUDE.md | 11 +-- frontend/package-lock.json | 16 +---- frontend/package.json | 3 +- frontend/src/App.contingency.test.tsx | 27 +++++++ frontend/src/App.tsx | 7 +- inspect_action.py | 38 ---------- 15 files changed, 189 insertions(+), 98 deletions(-) delete mode 100644 expert_backend/requirements.txt delete mode 100644 inspect_action.py diff --git a/.env.example b/.env.example index 61b13c77..63cf63e6 100644 --- a/.env.example +++ b/.env.example @@ -1,10 +1,14 @@ # Co-Study4Grid backend environment variables # Copy to `.env` (ignored by git) or export directly in your shell. -# Comma-separated list of allowed CORS origins (default: "*" wide-open dev mode). +# Comma-separated list of allowed CORS origins. Default (unset): the local +# Vite dev/preview origins on loopback (localhost/127.0.0.1 :5173 and :4173). # Set to the browser origin of the frontend in non-local deployments, e.g. # CORS_ALLOWED_ORIGINS=https://costudy.example.com,https://ops.example.com -CORS_ALLOWED_ORIGINS=* +# Wide-open dev mode is now explicit opt-in (a wildcard lets any web page read +# /api/* off a localhost backend): +# CORS_ALLOWED_ORIGINS=* +# CORS_ALLOWED_ORIGINS= # pypowsybl load-flow fast mode toggle (also set per-study from the UI). # Unset or empty = default; "1" / "true" = enabled; "0" / "false" = disabled. diff --git a/.github/workflows/deploy-huggingface.yml b/.github/workflows/deploy-huggingface.yml index 6d5add70..6e71cbc1 100644 --- a/.github/workflows/deploy-huggingface.yml +++ b/.github/workflows/deploy-huggingface.yml @@ -66,7 +66,7 @@ jobs: git add -A # Drop inert local dev artifacts from the deployed snapshot. git rm -q --cached --ignore-unmatch \ - .claude/plan.md inspect_action.py profiling_patch_results.json || true + .claude/plan.md profiling_patch_results.json || true git commit -q -m "Deploy ${GITHUB_SHA::7}" test "$(git rev-list --count HEAD)" -eq 1 # sanity: exactly one commit diff --git a/CLAUDE.md b/CLAUDE.md index 55292ccf..8bf420d0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,6 @@ Co-Study4Grid/ ├── expert_backend/ # Python FastAPI backend │ ├── CLAUDE.md # Backend-scoped guide (singletons, mixins, lifecycle) │ ├── main.py # FastAPI app: endpoints, CORS, gzip helpers, NDJSON streaming -│ ├── requirements.txt │ ├── test_backend.py # Ad-hoc integration script (not part of pytest) │ ├── recommenders/ # Pluggable recommendation-model registry: registry.py, │ │ │ # random_basic / random_overflow canonical examples, @@ -203,7 +202,6 @@ Co-Study4Grid/ - **Vite 7** - Build tool and dev server - **axios** - HTTP client - **react-select** - Searchable dropdown for branch selection -- **react-zoom-pan-pinch** - Pan/zoom for visualizations - **vite-plugin-singlefile** - Auto-generated single-file standalone bundle - **Vitest** + **React Testing Library** - Unit / integration tests @@ -402,13 +400,14 @@ NAD/SLD payloads: they sit. - **ViewBox zoom**: auto-centers on selected contingency targets with adjustable padding. -- **Pan/zoom**: `react-zoom-pan-pinch` in both the React dev build - and the auto-generated standalone (they share the same source - tree). +- **Pan/zoom**: the `usePanZoom` hook writes the SVG `viewBox` + directly (rAF-batched, cached CTM) — no pan/zoom library — in both + the React dev build and the auto-generated standalone (they share + the same source tree). ## Dependencies -### Backend (`expert_backend/requirements.txt` + `overrides.txt`) +### Backend (`pyproject.toml` + `overrides.txt`) - `fastapi`, `uvicorn`, `python-multipart` - `pypowsybl`, `expert_op4grid_recommender` (expected in venv) - `pandas>=2.2.2`, `numpy>=2.0.0`, `grid2op>=1.12.2`, `pandapower>=2.14.0` @@ -428,7 +427,7 @@ NAD/SLD payloads: ## Notes for AI Assistants - The backend API base URL defaults to `http://127.0.0.1:8000` in `frontend/src/api.ts` (`API_BASE_URL`), overridable at build time via `VITE_API_BASE_URL` — set it to `""` for **same-origin** hosting where the backend serves the SPA (the HuggingFace Docker Space), so requests become relative `/api/...` -- CORS is wide-open by default (`allow_origins=["*"]`) but configurable via the `CORS_ALLOWED_ORIGINS` env var (PR #104, see `.env.example`) +- CORS defaults to the local Vite dev/preview origins on loopback (`localhost`/`127.0.0.1` on `:5173` / `:4173`); a wildcard is explicit opt-in and any other set is configurable via the `CORS_ALLOWED_ORIGINS` env var (see `.env.example`) - **Frontend architecture (Phase 2 hook extraction, PR #109)**: `App.tsx` is the state orchestration hub; it must NOT contain large inline JSX blocks. Extracted presentational components live in `components/` and `components/modals/`; cross-cutting state pipelines live in `hooks/` (notably `useContingencyFetch` and `useDiagramHighlights`). When adding new UI sections, create a new component file (or hook for stateful pipelines) and wire it in `App.tsx`. - **`useSettings` hook**: Exposes a `SettingsState` object with all settings fields + setters. This is passed wholesale to `SettingsModal` to avoid excessive prop drilling. Adding a new setting means: (1) add to `useSettings.ts`, (2) add to `SettingsModal.tsx`. No manual standalone mirror is required — the legacy hand-maintained file has been decommissioned and the auto-generated bundle inherits from the React source automatically. - **Standalone bundle (auto-generated)**: `npm run build:standalone` in `frontend/` produces `frontend/dist-standalone/standalone.html` — a single-file HTML with React + CSS inlined via `vite-plugin-singlefile`. This is the canonical distribution artifact replacing the former `standalone_interface.html`. The legacy file remains on disk as `standalone_interface_legacy.html` (tracked as a frozen snapshot — do NOT edit). diff --git a/README.md b/README.md index 20c8d6c0..53ec1aa6 100644 --- a/README.md +++ b/README.md @@ -797,7 +797,6 @@ Co-Study4Grid consumes and produces. - **Vite 7** — build tool and dev server - **axios** — HTTP client - **react-select** — searchable dropdown for branch selection -- **react-zoom-pan-pinch** — pan/zoom for SVG visualizations - **vite-plugin-singlefile** — auto-generated single-file standalone bundle - **Vitest** + **React Testing Library** — unit tests (~1000 specs) diff --git a/docs/architecture/2026-07-full-repo-review.md b/docs/architecture/2026-07-full-repo-review.md index cf2a18db..cc8dbb3c 100644 --- a/docs/architecture/2026-07-full-repo-review.md +++ b/docs/architecture/2026-07-full-repo-review.md @@ -83,11 +83,15 @@ scoring function, annotated file trees vs. the tree). The five headline issues, all verified: -> **Remediation status (2026-07-07).** Headline #1 is **fixed** (D1), +> **Remediation status (2026-07-08).** Headline #1 is **fixed** (D1), > #3 is **fixed** (D3), and #2 is **partly fixed** (D2 — the error > contract + a machine-checked OpenAPI snapshot landed; TS-generation and > blanket-handler removal remain). #4 (invisible failures — D5) and #5 -> (trust model — D7) are still open. See Part V for the per-item detail. +> (trust model — D7) are still open at the deep-revision level, but their +> cheap quick-win halves landed 2026-07-08: analysis errors are now +> surfaced (QW4) and the two riskiest filesystem RPCs are hardened — +> loopback-default CORS (QW3) + a session-path traversal guard (QW7). +> See Part V for the per-item detail and Part VI for the quick wins. 1. **The pluggable-recommender subsystem is a ghost.** `expert_backend/recommenders/_service_integration.py` rewrites @@ -338,8 +342,8 @@ and five click grammars coexist.* | Sev | Finding | Verdict | |---|---|---| -| high | Analysis failures invisible: `useAnalysis`'s `error` state set on three failure paths, rendered nowhere; `App.tsx` destructures a *different* local `error` | ✅ | -| med | Re-simulation failures swallowed (`console.error` only) | ✅ | +| high | Analysis failures invisible: `useAnalysis`'s `error` state set on three failure paths, rendered nowhere; `App.tsx` destructures a *different* local `error` | ✅ → **RESOLVED (QW4, 2026-07-08)**: `StatusToasts` now renders `error \|\| analysis.error` (cleared on contingency-clear); App-integration test guards it. Root fix (one notification pipeline) is still D5. | +| med | Re-simulation failures swallowed (`console.error` only) | ✅ → **RESOLVED (QW4, 2026-07-08)**: the pin / SLD-edit catches now `setError`, and the SLD-preview catch was the last silent one — now surfaced too. | | med | Toast design: error banner has no dismiss & no timeout; info fixed 3 s; no `aria-live`; magic `'SUCCESS'` string protocol | ✅ | | med | Five single-vs-double-click grammars across surfaces; one documented backwards | 🟡 — grammars enumerated & confirmed; "backwards" doc claim narrowed | | med | No cancellation/abort for any long-running operation | ✅ | @@ -458,8 +462,8 @@ disabled under the wildcard; injected overlay renders untrusted data via | Sev | Finding | Verdict | |---|---|---| -| high | CORS `*` + no auth + arbitrary-file endpoints = drive-by local file read/write from any web page against a localhost install | ✅ | -| high | Unauthenticated arbitrary filesystem access survives on the public Space (Game Mode gates only the UI); `../` traversal in `session_name` | ✅ | +| high | CORS `*` + no auth + arbitrary-file endpoints = drive-by local file read/write from any web page against a localhost install | ✅ → **PARTLY RESOLVED (QW3, 2026-07-08)**: the CORS default is now loopback dev origins (wildcard is explicit opt-in), closing the drive-by cross-origin read. The no-auth/arbitrary-file surface itself is the D7 lockdown profile. | +| high | Unauthenticated arbitrary filesystem access survives on the public Space (Game Mode gates only the UI); `../` traversal in `session_name` | ✅ → **PARTLY RESOLVED (QW7, 2026-07-08)**: the `session_name` traversal is closed (`_safe_session_dir`). The broader unauthenticated-filesystem surface is the D7 lockdown profile. | | med | Singleton state shared across concurrent users, no locking (cross-ref T3) | ✅ → **RESOLVED (D3)**: service-level `RLock` + 409 study-mutation gate. | | med | `detail=str(e)` leaks absolute server paths | ✅ → **PARTLY RESOLVED (D2)**: the global unhandled-exception handler now returns a generic 500 (no `str(e)`); the per-endpoint `except → 400, str(e)` handlers still echo the message and are tracked for removal. | | med | `/api/pick-path` spawns a subprocess per request | 🟡 — headless child fails in ms (tkinter absent); low-severity dead weight, not a DoS vector | @@ -669,13 +673,13 @@ Day-one (each ≤ ~2 h, near-zero risk): |---|---|---| | QW1 | Collect root `tests/` (add to `pytest.ini` testpaths or move files) — *the highest value-per-hour change in this review* | pytest.ini / CI | | QW2 | ✅ **DONE (2026-07-07)** — `async def` → `def` on `run_analysis_step1` (unblocks the event loop; `TestEventLoopSafety` guard) | `main.py` | -| QW3 | CORS default `*` → loopback origins; wildcard becomes explicit opt-in | `main.py:141` | -| QW4 | Render `useAnalysis.error` in `StatusToasts`; add `setError` to the two swallowed catches | `App.tsx` | +| QW3 | ✅ **DONE (2026-07-08)** — CORS default `*` → loopback dev origins (`localhost`/`127.0.0.1` :5173/:4173); wildcard is explicit opt-in (`CORS_ALLOWED_ORIGINS="*"`) | `main.py` | +| QW4 | ✅ **DONE (2026-07-08)** — `useAnalysis.error` now surfaced through `StatusToasts` (`error \|\| analysis.error`, cleared on contingency-clear); the two `console.error`-only catches already carried `setError`, and the remaining SLD-preview catch got one. Guarded by a new App-integration test | `App.tsx` | | QW5 | ✅ **DONE (2026-07-07, folded into D3)** — try/finally on the unguarded variant switch | `diagram_mixin.py` `_get_contingency_flows` | | QW6 | Replace `detail=str(e)` with generic messages + server-side `logger.exception` | `main.py` | -| QW7 | Reject path separators/`..` in `session_name`; resolve+relative_to on session dirs (mirror the existing `/results/pdf` guard) | `main.py` | +| QW7 | ✅ **DONE (2026-07-08)** — `_safe_session_dir` rejects `..` / path separators / absolute names before any FS write, resolve()+relative_to() backstop (mirrors `/results/pdf`); applied to `save-session` + `load-session`. `TestSessionPathTraversal` guard | `main.py` | | QW8 | Pin `expert_op4grid_recommender` per-PR; float it in a separate canary job | CI ×3 + Dockerfile | -| QW9 | Delete `inspect_action.py`, dead `requirements.txt` stub, `react-zoom-pan-pinch`; untrack generated outputs | repo root / frontend | +| QW9 | 🟡 **PARTLY DONE (2026-07-08)** — deleted `inspect_action.py`, the dead `expert_backend/requirements.txt` stub, and the unused `react-zoom-pan-pinch` dep (+ doc/lockfile follow-through). Did **not** untrack the `Overflow_Graph/*.html`: it is a skip-guarded regression fixture (`test_overflow_html_dim_logic.py`) — untracking it would silently drop that CI coverage | repo root / frontend | Week-one (½–1 day each): diff --git a/expert_backend/CLAUDE.md b/expert_backend/CLAUDE.md index 232a15fa..7bad0fc0 100644 --- a/expert_backend/CLAUDE.md +++ b/expert_backend/CLAUDE.md @@ -17,7 +17,6 @@ expert_backend/ ├── __init__.py ├── main.py # FastAPI app: endpoints, CORS, gzip helpers, │ # config-file persistence, NDJSON streaming -├── requirements.txt # Pinned core deps (fastapi, uvicorn, multipart) ├── test_backend.py # Ad-hoc integration script (not part of pytest) ├── services/ │ ├── __init__.py @@ -528,7 +527,11 @@ python -m expert_backend.main uvicorn expert_backend.main:app --host 0.0.0.0 --port 8000 ``` -CORS defaults to wide-open (`allow_origins=["*"]`) because the dev -frontend hits the backend cross-origin. It is configurable via the -`CORS_ALLOWED_ORIGINS` env var (PR #104 — see `.env.example`). -Tighten before any non-local deployment. +CORS defaults to the local Vite dev/preview origins on loopback +(`localhost` / `127.0.0.1` on `:5173` and `:4173`) so the dev frontend +can hit the backend cross-origin without opening it to every web page. +A wildcard (`allow_origins=["*"]`) is now explicit opt-in via +`CORS_ALLOWED_ORIGINS="*"`; a comma-separated list overrides the default +for a specific deployment (see `.env.example`). The same-origin +HuggingFace Space needs no entry (same-origin requests don't trigger +CORS). diff --git a/expert_backend/main.py b/expert_backend/main.py index 197496dd..371c97e8 100644 --- a/expert_backend/main.py +++ b/expert_backend/main.py @@ -160,10 +160,26 @@ def _save_user_config(data: dict) -> None: _ensure_user_config() -_CORS_ENV = os.environ.get("CORS_ALLOWED_ORIGINS", "*").strip() -_CORS_ORIGINS = ( - ["*"] if _CORS_ENV == "*" else [o.strip() for o in _CORS_ENV.split(",") if o.strip()] -) +# CORS. The dev frontend (Vite) hits the backend cross-origin, so some +# origins must be allowed — but a wildcard *default* is a drive-by +# local-file-read vector: any web page the operator happens to visit +# could read `/api/*` (including the filesystem RPCs) off the localhost +# backend. So the default is now the local dev-server origins on +# loopback; the wildcard is explicit opt-in via CORS_ALLOWED_ORIGINS="*". +# A comma-separated list overrides both for a specific deployment. The +# same-origin HuggingFace Space needs no entry here (same-origin requests +# don't trigger CORS). +_CORS_DEFAULT_ORIGINS = [ + "http://localhost:5173", "http://127.0.0.1:5173", # Vite dev server + "http://localhost:4173", "http://127.0.0.1:4173", # Vite preview +] +_CORS_ENV = os.environ.get("CORS_ALLOWED_ORIGINS", "").strip() +if _CORS_ENV == "*": + _CORS_ORIGINS = ["*"] +elif _CORS_ENV: + _CORS_ORIGINS = [o.strip() for o in _CORS_ENV.split(",") if o.strip()] +else: + _CORS_ORIGINS = _CORS_DEFAULT_ORIGINS app.add_middleware( CORSMiddleware, allow_origins=_CORS_ORIGINS, @@ -562,6 +578,27 @@ def _pick_path_tkinter(kind: str) -> dict: return {"path": "", "error": err} return {"path": proc.stdout.strip()} +def _safe_session_dir(base_folder: str, session_name: str) -> str: + """Resolve ``/`` while rejecting any + ``session_name`` that escapes ``base_folder`` — path separators, + ``..``, or an absolute path (``os.path.join`` silently drops the base + when the second arg is absolute). Mirrors the ``/results/pdf`` + traversal guard. Returns the resolved absolute session directory.""" + name = (session_name or "").strip() + # A session name is a single path component. basename() strips any + # directory part; '.' / '..' are rejected explicitly (basename keeps + # them). The resolve()+relative_to() below is the defense-in-depth + # backstop that also catches separators basename doesn't split on. + if not name or name in (".", "..") or os.path.basename(name) != name: + raise HTTPException(status_code=400, detail="Invalid session name") + base = Path(base_folder).resolve() + candidate = (base / name).resolve() + try: + candidate.relative_to(base) + except ValueError: + raise HTTPException(status_code=400, detail="Invalid session name") + return str(candidate) + @app.post("/api/save-session", response_model=SaveSessionResponse) def save_session(request: SaveSessionRequest) -> dict: import shutil @@ -569,7 +606,7 @@ def save_session(request: SaveSessionRequest) -> dict: if not request.output_folder_path: raise HTTPException(status_code=400, detail="output_folder_path is required") - session_dir = os.path.join(request.output_folder_path, request.session_name) + session_dir = _safe_session_dir(request.output_folder_path, request.session_name) try: os.makedirs(session_dir, exist_ok=True) except OSError as e: @@ -626,7 +663,7 @@ def load_session(folder_path: str = Body(...), session_name: str = Body(...)) -> import shutil import glob - session_dir = os.path.join(folder_path, session_name) + session_dir = _safe_session_dir(folder_path, session_name) json_path = os.path.join(session_dir, "session.json") if not os.path.isfile(json_path): diff --git a/expert_backend/requirements.txt b/expert_backend/requirements.txt deleted file mode 100644 index a9c81efa..00000000 --- a/expert_backend/requirements.txt +++ /dev/null @@ -1,5 +0,0 @@ -fastapi -uvicorn -python-multipart -lxml -# pypowsybl and expert_op4grid_recommender are already in the venv diff --git a/expert_backend/tests/test_api_endpoints.py b/expert_backend/tests/test_api_endpoints.py index bb658f53..c82652a3 100644 --- a/expert_backend/tests/test_api_endpoints.py +++ b/expert_backend/tests/test_api_endpoints.py @@ -1590,6 +1590,76 @@ def test_save_session_response_exact_fields(self, client, mock_services, tmp_pat assert resp.json()["pdf_copied"] is False +class TestSessionPathTraversal: + """`session_name` is joined onto a caller-supplied folder to build the + session directory, so it must be a single path component — a `..`, a + path separator, or an absolute path (which `os.path.join` honours, + silently dropping the base) would let save/load escape the output + folder (QW7, 2026-07). `_safe_session_dir` rejects those with a 400 + before any filesystem write.""" + + def test_save_session_rejects_parent_traversal(self, client, mock_services, tmp_path): + base = tmp_path / "sessions" + base.mkdir() + resp = client.post( + "/api/save-session", + json={ + "session_name": "../escaped", + "json_content": "{}", + "output_folder_path": str(base), + }, + ) + assert resp.status_code == 400 + assert resp.json()["detail"] == "Invalid session name" + # Nothing was written outside the base folder. + assert not (tmp_path / "escaped").exists() + + def test_save_session_rejects_path_separator(self, client, mock_services, tmp_path): + resp = client.post( + "/api/save-session", + json={ + "session_name": "nested/child", + "json_content": "{}", + "output_folder_path": str(tmp_path), + }, + ) + assert resp.status_code == 400 + assert not (tmp_path / "nested").exists() + + def test_save_session_rejects_absolute_name(self, client, mock_services, tmp_path): + evil = tmp_path / "abs_escape" + resp = client.post( + "/api/save-session", + json={ + "session_name": str(evil), + "json_content": "{}", + "output_folder_path": str(tmp_path / "base"), + }, + ) + assert resp.status_code == 400 + assert not evil.exists() + + def test_load_session_rejects_parent_traversal(self, client, mock_services, tmp_path): + resp = client.post( + "/api/load-session", + json={"folder_path": str(tmp_path), "session_name": "../../etc"}, + ) + assert resp.status_code == 400 + assert resp.json()["detail"] == "Invalid session name" + + def test_save_session_allows_plain_name(self, client, mock_services, tmp_path): + resp = client.post( + "/api/save-session", + json={ + "session_name": "costudy4grid_session_ok", + "json_content": "{}", + "output_folder_path": str(tmp_path), + }, + ) + assert resp.status_code == 200 + assert (tmp_path / "costudy4grid_session_ok" / "session.json").is_file() + + class TestEventLoopSafety: """`/api/run-analysis-step1` runs seconds of synchronous pypowsybl / grid2op work, so it MUST be a sync `def` route (dispatched to diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 7277739e..c6452da7 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -17,7 +17,7 @@ frontend/ ├── README.md ├── index.html # Vite HTML entry point ├── package.json # React 19, Vite 7, vitest, axios, react-select, -│ # react-zoom-pan-pinch, vite-plugin-singlefile +│ # vite-plugin-singlefile ├── vite.config.ts # Vite + Vitest plugin (jsdom env) ├── eslint.config.js # Flat config (v9+) — typescript-eslint, │ # react-hooks, react-refresh @@ -395,9 +395,10 @@ performance levers are applied today: on the ~12 MB French NAD. Falls back to the full NAD on any unsupported edge case. -The visualization is rendered inside `react-zoom-pan-pinch`. Zoom -state is owned by `usePanZoom` per tab (`nPZ`, `n1PZ`, `actionPZ`, -plus `overviewPz` for the Action overview map). The +Pan/zoom is implemented directly by `usePanZoom` — it writes the SVG +`viewBox` on the container (rAF-batched, cached CTM), with no pan/zoom +library. Zoom state is owned by `usePanZoom` per tab (`nPZ`, `n1PZ`, +`actionPZ`, plus `overviewPz` for the Action overview map). The `useTiedTabsSync` hook mirrors viewBox changes from the active tab to any "tied" detached tab. @@ -466,7 +467,7 @@ selector in Settings → Configurations, default `'off'`). See `useDetachedTabs` opens diagram tabs in popup windows (`window.open`). When popups are blocked, the error surfaces via `onPopupBlocked` callback. The detached tab gets its own -`react-zoom-pan-pinch` instance; `useTiedTabsSync` keeps the +`usePanZoom` viewBox state; `useTiedTabsSync` keeps the viewBoxes in sync. See `docs/features/detachable-viz-tabs.md`. ## Interaction logging diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2764a4a9..81110ac6 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -11,8 +11,7 @@ "axios": "^1.13.5", "react": "^19.2.0", "react-dom": "^19.2.0", - "react-select": "^5.10.2", - "react-zoom-pan-pinch": "^3.7.0" + "react-select": "^5.10.2" }, "devDependencies": { "@eslint/js": "^9.39.1", @@ -4345,19 +4344,6 @@ "react-dom": ">=16.6.0" } }, - "node_modules/react-zoom-pan-pinch": { - "version": "3.7.0", - "resolved": "https://registry.npmjs.org/react-zoom-pan-pinch/-/react-zoom-pan-pinch-3.7.0.tgz", - "integrity": "sha512-UmReVZ0TxlKzxSbYiAj+LeGRW8s8LraAFTXRAxzMYnNRgGPsxCudwZKVkjvGmjtx7SW/hZamt69NUmGf4xrkXA==", - "engines": { - "node": ">=8", - "npm": ">=5" - }, - "peerDependencies": { - "react": "*", - "react-dom": "*" - } - }, "node_modules/redent": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/redent/-/redent-3.0.0.tgz", diff --git a/frontend/package.json b/frontend/package.json index 58b87ab8..cb0c5cea 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -19,8 +19,7 @@ "axios": "^1.13.5", "react": "^19.2.0", "react-dom": "^19.2.0", - "react-select": "^5.10.2", - "react-zoom-pan-pinch": "^3.7.0" + "react-select": "^5.10.2" }, "devDependencies": { "@eslint/js": "^9.39.1", diff --git a/frontend/src/App.contingency.test.tsx b/frontend/src/App.contingency.test.tsx index 1ebe5982..72e2ef9f 100644 --- a/frontend/src/App.contingency.test.tsx +++ b/frontend/src/App.contingency.test.tsx @@ -326,6 +326,33 @@ describe('Contingency Change Confirmation', () => { await userEvent.type(combobox, 'INVALID_NAME'); expect(screen.queryByText('Change Contingency?')).not.toBeInTheDocument(); }); + + it('surfaces an analysis stream error in the status toast (QW4)', async () => { + await renderAndLoadStudy(); + await selectBranch('BRANCH_A'); + + // Step 2 emits an `error` event. `useAnalysis` records it on its OWN + // `error` state (not `infoMessage`), which nothing rendered before + // QW4; the fix wires that hook error into the shared StatusToasts + // channel so an analysis failure is no longer a silent dead spinner. + const errStream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode( + JSON.stringify({ type: 'error', message: 'Backend crashed' }) + '\n' + )); + controller.close(); + }, + }); + mockApi.runAnalysisStep2Stream.mockResolvedValue({ ok: true, body: errStream }); + + await act(async () => { + await userEvent.click(screen.getByText('🔍 Analyze & Suggest')); + }); + + expect( + await screen.findByText(/Analysis failed: Backend crashed/, {}, { timeout: 3000 }) + ).toBeInTheDocument(); + }); }); describe('Overload Clearing Logic', () => { diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b871c7eb..3477b5d1 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -328,6 +328,10 @@ function App() { // toggle matches the backend's freshly-cleared overflow cache. diagrams.setOverflowLayoutMode('hierarchical'); setError(''); + // The analysis hook owns its own `error` (surfaced via the shared + // StatusToasts channel — see the render site). Clear it here too so + // a failed-analysis banner doesn't survive a contingency clear. + analysis.setError(''); analysis.setInfoMessage(''); diagrams.setInspectQuery(''); // Do NOT reset lastZoomState here. Resetting it causes the auto-zoom @@ -743,6 +747,7 @@ function App() { if (seq !== sldPreviewSeqRef.current) return; console.error('SLD topology preview failed:', e); setSldPreview(null); + setError(apiErrorMessage(e, 'Topology preview failed')); } finally { if (seq === sldPreviewSeqRef.current) setSldPreviewLoading(false); } @@ -2078,7 +2083,7 @@ function App() { onCancel={handleCancelDialog} onConfirm={handleConfirmDialog} /> - + ); } diff --git a/inspect_action.py b/inspect_action.py deleted file mode 100644 index 7596ee21..00000000 --- a/inspect_action.py +++ /dev/null @@ -1,38 +0,0 @@ - -import sys -import os -import numpy as np - -# Mocking enough to run RecommenderService -os.environ["GRID2OP_DATA"] = "/tmp" - -from expert_backend.services.recommender_service import RecommenderService -from unittest.mock import MagicMock, patch - -def inspect_action(): - svc = RecommenderService() - - # We need a real-ish env if possible, or just mock it but check what the code expects - env = MagicMock() - obs = MagicMock() - obs.name_gen = ["GEN_A"] - obs.gen_p = np.array([10.0]) - obs.name_line = ["LINE_1"] - obs.rho = np.array([0.1]) - obs.n_components = 1 - env.get_obs.return_value = obs - - # Simulate first call - with patch.object(svc, "_get_simulation_env", return_value=env), \ - patch.object(svc, "_get_monitoring_parameters", return_value=(set(), set())), \ - patch.object(svc, "_compute_deltas", return_value={}): - - # This will call env.action_space(content) - # We want to see what the REAL grid2op returns, but here we mock it. - # Wait, if I mock it, I'm just testing my mock. - - # Instead, let's look at recommender_service.py and see if there's a better way to get the setpoint. - pass - -if __name__ == "__main__": - print("This script is a placeholder to remind me to check the code instead of mocking.") From ecb060f90aa7656d0c5099bbdecccf2f07499f37 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Wed, 8 Jul 2026 20:20:18 +0000 Subject: [PATCH 02/24] test+docs: extend QW-bundle coverage + reconcile related docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows up the QW3/4/7/9 bundle with tests and documentation: - QW3: extract `_resolve_cors_origins` (testable) and add `TestCorsDefault` — unit cases (unset/empty → loopback, `*` opt-in, comma list, credentials invariant) plus TestClient integration proving a disallowed Origin gets no ACAO header while a loopback Origin is reflected. - QW7: add a `load-session` happy-path case alongside the traversal rejections so the guard is shown not to break legitimate loads. - QW4: add an App-integration test that a surfaced analysis error is dismissed when the contingency is cleared. - Docs: document the `session_name` path-validation contract in save-results.md; note the new TestSessionPathTraversal / TestCorsDefault classes in the test-suite guide. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- docs/features/save-results.md | 13 +++++ expert_backend/main.py | 22 +++++--- expert_backend/tests/CLAUDE.md | 2 +- expert_backend/tests/test_api_endpoints.py | 66 ++++++++++++++++++++++ frontend/src/App.contingency.test.tsx | 33 +++++++++++ 5 files changed, 128 insertions(+), 8 deletions(-) diff --git a/docs/features/save-results.md b/docs/features/save-results.md index 3828470d..cb5140ee 100644 --- a/docs/features/save-results.md +++ b/docs/features/save-results.md @@ -391,6 +391,19 @@ Returns `{ "sessions": ["session_name_1", "session_name_2", ...] }` sorted most- Returns the parsed `session.json` content. Also restores the overflow PDF to `Overflow_Graph/` if it was removed since saving. +#### Path validation (`session_name`) + +`session_name` is joined onto the caller-supplied folder to build the +session directory, so both `save-session` and `load-session` route it +through `_safe_session_dir` (`main.py`) first. It must be a **single +path component**: a `..`, a path separator, or an absolute path (which +`os.path.join` silently honours, dropping the base) is rejected with +**HTTP 400 `{"detail": "Invalid session name", "code": "BAD_REQUEST"}`** +before any filesystem access, backed by a `resolve()`+`relative_to()` +containment check that mirrors the `/results/pdf` traversal guard (QW7, +2026-07). Regression-guarded by `TestSessionPathTraversal` in +`expert_backend/tests/test_api_endpoints.py`. + ### Restore Analysis Context `POST /api/restore-analysis-context` — Re-push the session's monitored-line set and computed-pair cache into the backend service's `_analysis_context` so any subsequent `simulate-manual-action` / `compute-superposition` call on the reloaded session uses the same policy as the original study. Called from `useSession::handleRestoreSession` right after the base-diagram `Promise.all` (step 4 of the reload flow). diff --git a/expert_backend/main.py b/expert_backend/main.py index 371c97e8..96da5c99 100644 --- a/expert_backend/main.py +++ b/expert_backend/main.py @@ -173,13 +173,21 @@ def _save_user_config(data: dict) -> None: "http://localhost:5173", "http://127.0.0.1:5173", # Vite dev server "http://localhost:4173", "http://127.0.0.1:4173", # Vite preview ] -_CORS_ENV = os.environ.get("CORS_ALLOWED_ORIGINS", "").strip() -if _CORS_ENV == "*": - _CORS_ORIGINS = ["*"] -elif _CORS_ENV: - _CORS_ORIGINS = [o.strip() for o in _CORS_ENV.split(",") if o.strip()] -else: - _CORS_ORIGINS = _CORS_DEFAULT_ORIGINS + + +def _resolve_cors_origins(env_value: str | None) -> list[str]: + """Map the ``CORS_ALLOWED_ORIGINS`` env value to an allow-list: + ``"*"`` → wildcard (explicit opt-in), a comma-separated list → that + list, and unset/empty → the loopback dev-server default.""" + env = (env_value or "").strip() + if env == "*": + return ["*"] + if env: + return [o.strip() for o in env.split(",") if o.strip()] + return list(_CORS_DEFAULT_ORIGINS) + + +_CORS_ORIGINS = _resolve_cors_origins(os.environ.get("CORS_ALLOWED_ORIGINS")) app.add_middleware( CORSMiddleware, allow_origins=_CORS_ORIGINS, diff --git a/expert_backend/tests/CLAUDE.md b/expert_backend/tests/CLAUDE.md index d9403645..a599ad2e 100644 --- a/expert_backend/tests/CLAUDE.md +++ b/expert_backend/tests/CLAUDE.md @@ -44,7 +44,7 @@ Configuration in `frontend/vite.config.ts` (Vitest plugin). #### API & Service Layer | File | Description | |------|-------------| -| `test_api_endpoints.py` | FastAPI endpoint testing with TestClient and mocked services (covers the patch endpoints `/api/contingency-diagram-patch`, `/api/action-variant-diagram-patch`, `/api/simulate-and-variant-diagram`; plus `TestStudyMutationBusyGate` for the D3 HTTP-409 gate, `TestResponseModels` for the D2 response-model field-completeness, and `TestEventLoopSafety` for the QW2 sync-`def` guard). | +| `test_api_endpoints.py` | FastAPI endpoint testing with TestClient and mocked services (covers the patch endpoints `/api/contingency-diagram-patch`, `/api/action-variant-diagram-patch`, `/api/simulate-and-variant-diagram`; plus `TestStudyMutationBusyGate` for the D3 HTTP-409 gate, `TestResponseModels` for the D2 response-model field-completeness, `TestEventLoopSafety` for the QW2 sync-`def` guard, `TestSessionPathTraversal` for the QW7 `session_name` traversal guard, and `TestCorsDefault` for the QW3 loopback-default CORS + `_resolve_cors_origins`). | | `test_api_errors.py` | **D2 error contract** (`services/api_errors.py`): `AppHTTPException` + `_code_for` mapping, the three handlers, the security-critical "uncaught exception → generic 500, NO `str(e)` leak", the `ACTION_RESULT_UNAVAILABLE` discriminator reaching the client, and a response-validation-failure → generic-500 integration proof. | | `test_openapi_contract.py` | **D2 machine-check**: the live `app.openapi()` must equal the committed `openapi.snapshot.json` (regenerate with `check_openapi_contract.py --write`), plus the `{detail, code}` envelope on 422 / 404. | | `test_service_concurrency.py` | **D3 concurrency ownership** (`services/service_lock.py` + service wiring): re-entrant network lock, cross-thread study-gate release + HTTP-409 gate, `@with_network_lock[_stream]` serialization + per-`next()` lock release + no-op fallback, the contingency-variant LRU (eviction / reuse-reorder / never-evict-working / reset clears it), and the NAD-prefetch `_prefetch_generation` staleness discard (the lock-ordering deadlock fix). | diff --git a/expert_backend/tests/test_api_endpoints.py b/expert_backend/tests/test_api_endpoints.py index c82652a3..31e3b1b6 100644 --- a/expert_backend/tests/test_api_endpoints.py +++ b/expert_backend/tests/test_api_endpoints.py @@ -1659,6 +1659,72 @@ def test_save_session_allows_plain_name(self, client, mock_services, tmp_path): assert resp.status_code == 200 assert (tmp_path / "costudy4grid_session_ok" / "session.json").is_file() + def test_load_session_allows_plain_name(self, client, mock_services, tmp_path): + # The guard must not break a legitimate round-trip. + import json as _json + sess = tmp_path / "costudy4grid_session_ok" + sess.mkdir() + (sess / "session.json").write_text(_json.dumps({"hello": "world"})) + resp = client.post( + "/api/load-session", + json={"folder_path": str(tmp_path), "session_name": "costudy4grid_session_ok"}, + ) + assert resp.status_code == 200 + assert resp.json()["hello"] == "world" + + +class TestCorsDefault: + """CORS defaults to the loopback dev-server origins, NOT a wildcard + (QW3, 2026-07): a `*` default lets any web page the operator visits + read `/api/*` (incl. the filesystem RPCs) off the localhost backend, + so the wildcard is now explicit opt-in via `CORS_ALLOWED_ORIGINS`.""" + + def test_unset_env_resolves_to_loopback_not_wildcard(self): + from expert_backend import main + origins = main._resolve_cors_origins(None) + assert "*" not in origins + assert origins == main._CORS_DEFAULT_ORIGINS + assert "http://localhost:5173" in origins + assert "http://127.0.0.1:5173" in origins + + def test_empty_env_resolves_to_loopback(self): + from expert_backend import main + assert main._resolve_cors_origins(" ") == main._CORS_DEFAULT_ORIGINS + + def test_explicit_wildcard_is_opt_in(self): + from expert_backend import main + assert main._resolve_cors_origins("*") == ["*"] + + def test_comma_separated_list_overrides(self): + from expert_backend import main + assert main._resolve_cors_origins("https://a.example, https://b.example") == [ + "https://a.example", + "https://b.example", + ] + + def test_live_app_default_is_not_wildcard(self): + # The app is configured at import from the (unset in tests) env. + from expert_backend import main + assert main._CORS_ORIGINS != ["*"] + + def test_credentials_invariant(self): + # The middleware sets allow_credentials = (origins != ["*"]): + # ON for a concrete allow-list, OFF under the wildcard (browsers + # reject `*` + credentials anyway). + from expert_backend import main + assert (main._resolve_cors_origins("") != ["*"]) is True + assert (main._resolve_cors_origins("*") != ["*"]) is False + + def test_disallowed_origin_gets_no_cors_header(self, client): + resp = client.get("/api/user-config", headers={"Origin": "http://evil.example"}) + acao = resp.headers.get("access-control-allow-origin") + assert acao != "*" + assert acao != "http://evil.example" + + def test_allowed_loopback_origin_is_reflected(self, client): + resp = client.get("/api/user-config", headers={"Origin": "http://localhost:5173"}) + assert resp.headers.get("access-control-allow-origin") == "http://localhost:5173" + class TestEventLoopSafety: """`/api/run-analysis-step1` runs seconds of synchronous pypowsybl / diff --git a/frontend/src/App.contingency.test.tsx b/frontend/src/App.contingency.test.tsx index 72e2ef9f..c2bf73b0 100644 --- a/frontend/src/App.contingency.test.tsx +++ b/frontend/src/App.contingency.test.tsx @@ -353,6 +353,39 @@ describe('Contingency Change Confirmation', () => { await screen.findByText(/Analysis failed: Backend crashed/, {}, { timeout: 3000 }) ).toBeInTheDocument(); }); + + it('dismisses a surfaced analysis error when the contingency is cleared (QW4)', async () => { + await renderAndLoadStudy(); + await selectBranch('BRANCH_A'); + + const errStream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode( + JSON.stringify({ type: 'error', message: 'Backend crashed' }) + '\n' + )); + controller.close(); + }, + }); + mockApi.runAnalysisStep2Stream.mockResolvedValue({ ok: true, body: errStream }); + await act(async () => { + await userEvent.click(screen.getByText('🔍 Analyze & Suggest')); + }); + await screen.findByText(/Analysis failed: Backend crashed/, {}, { timeout: 3000 }); + + // Clearing the contingency must also clear the analysis-hook error — + // otherwise the failure banner would outlive the state it describes. + await act(async () => { + await userEvent.click(screen.getByTestId('sidebar-summary-clear')); + }); + const maybeConfirm = screen.queryByText('Confirm'); + if (maybeConfirm) { + await act(async () => { await userEvent.click(maybeConfirm); }); + } + + await waitFor(() => { + expect(screen.queryByText(/Analysis failed: Backend crashed/)).not.toBeInTheDocument(); + }, { timeout: 3000 }); + }); }); describe('Overload Clearing Logic', () => { From 2b5a158b7608274c04212d35e312278dc32d71e3 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Wed, 8 Jul 2026 20:31:50 +0000 Subject: [PATCH 03/24] refactor(D5.1/QW10): single utils/ndjsonStream.ts, replace 5 parser copies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NDJSON streaming body was parsed by five hand-copied reader loops (App.tsx x2, useAnalysis, useDiagrams, ActionFeed) that had drifted: only the two App.tsx copies flushed a final line lacking a trailing newline, and each re-implemented blank-line / parse-error handling slightly differently. `parseNdjsonStream(source, {signal?})` is now the one implementation — an async generator that decodes with stream:true (carrying the partial line across chunks), yields one parsed value per complete line, skips blank / unparseable lines, flushes a trailing unterminated final line on end, and honours an AbortSignal (cancels the reader, ends iteration without flushing the partial buffer — foundation for D5.3 cancellation). Reader cancellation in cleanup is defensive so it never throws out of a generator finally. All five callers now consume it via `for await`. Behaviour-preserving (full Vitest suite green); covered by src/utils/ndjsonStream.test.ts. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- frontend/src/App.tsx | 79 +++------------- frontend/src/components/ActionFeed.tsx | 39 +++----- frontend/src/hooks/useAnalysis.ts | 116 +++++++++++------------- frontend/src/hooks/useDiagrams.ts | 105 +++++++++------------ frontend/src/utils/ndjsonStream.test.ts | 116 ++++++++++++++++++++++++ frontend/src/utils/ndjsonStream.ts | 107 ++++++++++++++++++++++ 6 files changed, 348 insertions(+), 214 deletions(-) create mode 100644 frontend/src/utils/ndjsonStream.test.ts create mode 100644 frontend/src/utils/ndjsonStream.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 3477b5d1..13b620a7 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -35,6 +35,7 @@ import { gameBridge } from './game/gameBridge'; import type { GameStudy } from './game/types'; import { DEFAULT_ACTION_OVERVIEW_FILTERS } from './utils/actionTypes'; import { apiErrorMessage } from './utils/apiError'; +import { parseNdjsonStream } from './utils/ndjsonStream'; import { attachVlInteractions } from './utils/svgUtils'; import { buildOverflowPinPayload, @@ -589,54 +590,20 @@ function App() { target_mw: null, target_tap: null, }); - const reader = response.body!.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; let metrics: Awaited> | null = null; let streamErr: string | null = null; - while (true) { - const { value, done } = await reader.read(); - if (done) { - // Flush any trailing content that lacked a final \n. - // Backend always appends \n today, but this guard keeps - // the path robust if a future change emits a final - // event without one. - if (buffer.trim()) { - try { - const event = JSON.parse(buffer) as Record; - if (event.type === 'metrics') { - const { type: _t, ...rest } = event; - void _t; - metrics = rest as Awaited>; - } else if (event.type === 'diagram') { - const { type: _t, ...rest } = event; - void _t; - diagrams.primeActionDiagram(actionId, rest as unknown as DiagramData & { svg: string }, voltageLevels.length); - } else if (event.type === 'error') { - streamErr = (event.message as string) || 'stream error'; - } - } catch { /* ignore malformed trailing bytes */ } - } - break; - } - buffer += decoder.decode(value, { stream: true }); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) { - if (!line.trim()) continue; - let event: Record; - try { event = JSON.parse(line); } catch { continue; } - if (event.type === 'metrics') { - const { type: _t, ...rest } = event; - void _t; - metrics = rest as Awaited>; - } else if (event.type === 'diagram') { - const { type: _t, ...rest } = event; - void _t; - diagrams.primeActionDiagram(actionId, rest as unknown as DiagramData & { svg: string }, voltageLevels.length); - } else if (event.type === 'error') { - streamErr = (event.message as string) || 'stream error'; - } + for await (const raw of parseNdjsonStream(response)) { + const event = raw as Record; + if (event.type === 'metrics') { + const { type: _t, ...rest } = event; + void _t; + metrics = rest as Awaited>; + } else if (event.type === 'diagram') { + const { type: _t, ...rest } = event; + void _t; + diagrams.primeActionDiagram(actionId, rest as unknown as DiagramData & { svg: string }, voltageLevels.length); + } else if (event.type === 'error') { + streamErr = (event.message as string) || 'stream error'; } } if (streamErr) throw new Error(streamErr); @@ -807,15 +774,10 @@ function App() { target_tap: null, voltage_level_id: vlName, }); - const reader = response.body!.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; let metrics: Awaited> | null = null; let streamErr: string | null = null; - const parseEvent = (line: string) => { - if (!line.trim()) return; - let event: Record; - try { event = JSON.parse(line); } catch { return; } + for await (const raw of parseNdjsonStream(response)) { + const event = raw as Record; if (event.type === 'metrics') { const { type: _t, ...rest } = event; void _t; @@ -832,17 +794,6 @@ function App() { } else if (event.type === 'error') { streamErr = (event.message as string) || 'stream error'; } - }; - while (true) { - const { value, done } = await reader.read(); - if (done) { - if (buffer.trim()) parseEvent(buffer); - break; - } - buffer += decoder.decode(value, { stream: true }); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) parseEvent(line); } if (streamErr) throw new Error(streamErr); if (!metrics) throw new Error('Stream ended without metrics event'); diff --git a/frontend/src/components/ActionFeed.tsx b/frontend/src/components/ActionFeed.tsx index 2550abd8..3918b94b 100644 --- a/frontend/src/components/ActionFeed.tsx +++ b/frontend/src/components/ActionFeed.tsx @@ -12,6 +12,7 @@ import { DEFAULT_ACTION_OVERVIEW_FILTERS, matchesActionTypeFilter, rowPassesActi import { api } from '../api'; import type { ModelDescriptor } from '../api'; import { interactionLogger } from '../utils/interactionLogger'; +import { parseNdjsonStream } from '../utils/ndjsonStream'; import CombinedActionsModal from './CombinedActionsModal'; import ActionCard from './ActionCard'; import AdditionalLinesPicker from './AdditionalLinesPicker'; @@ -242,33 +243,21 @@ const ActionFeed: React.FC = ({ target_mw: targetMw ?? null, target_tap: targetTap ?? null, }); - const reader = response.body!.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; let metrics: Awaited> | null = null; let streamErr: string | null = null; - while (true) { - const { value, done } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) { - if (!line.trim()) continue; - let event: Record; - try { event = JSON.parse(line); } catch { continue; } - if (event.type === 'metrics') { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { type: _t, ...rest } = event; - metrics = rest as Awaited>; - } else if (event.type === 'diagram') { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { type: _t, ...rest } = event; - // `canPrimeDiagram` is verified before this helper is called. - onActionDiagramPrimed!(actionId, rest as unknown as DiagramData & { svg: string }, voltageLevelsLength!); - } else if (event.type === 'error') { - streamErr = (event.message as string) || 'stream error'; - } + for await (const raw of parseNdjsonStream(response)) { + const event = raw as Record; + if (event.type === 'metrics') { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { type: _t, ...rest } = event; + metrics = rest as Awaited>; + } else if (event.type === 'diagram') { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { type: _t, ...rest } = event; + // `canPrimeDiagram` is verified before this helper is called. + onActionDiagramPrimed!(actionId, rest as unknown as DiagramData & { svg: string }, voltageLevelsLength!); + } else if (event.type === 'error') { + streamErr = (event.message as string) || 'stream error'; } } if (streamErr) throw new Error(streamErr); diff --git a/frontend/src/hooks/useAnalysis.ts b/frontend/src/hooks/useAnalysis.ts index d8bcad17..c57f8978 100644 --- a/frontend/src/hooks/useAnalysis.ts +++ b/frontend/src/hooks/useAnalysis.ts @@ -8,6 +8,7 @@ import { useState, useEffect, useCallback, useMemo, useRef, type Dispatch, type SetStateAction, type MutableRefObject } from 'react'; import type { ActionDetail, AnalysisResult, TabId } from '../types'; import { interactionLogger } from '../utils/interactionLogger'; +import { parseNdjsonStream } from '../utils/ndjsonStream'; export interface AnalysisState { result: AnalysisResult | null; @@ -174,73 +175,58 @@ export function useAnalysis(): AnalysisState { additional_lines_to_cut: additionalLinesArr, }); - const reader = response2.body!.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; let step2ActionsCount = 0; - while (true) { - const { value, done } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) { - if (!line.trim()) continue; - try { - const event = JSON.parse(line); - if (event.type === 'pdf') { - setResult((p: AnalysisResult | null) => ({ - ...(p || {}), - pdf_url: event.pdf_url, - pdf_path: event.pdf_path, - // The overflow-graph build time is emitted on the - // ``pdf`` event (before discovery + assessment finish) - // so the iframe can display it below its title as soon - // as the file is ready. - ...(typeof event.overflow_graph_time === 'number' - ? { overflow_graph_time: event.overflow_graph_time } - : (event.overflow_graph_time === null - ? { overflow_graph_time: null } - : {})), - } as AnalysisResult)); - if (setActiveTab) { - setActiveTab('overflow'); - } - } else if (event.type === 'result') { - const actionsWithFlags = { ...event.actions }; - // Provenance for recommender-produced actions: the model - // the backend actually ran (echoed as `active_model` on - // the result event). Preserve an existing `origin` so a - // "first guess" the operator added before analysis keeps - // its `"user"` provenance even when the model re-emits - // the same id. - const resultModel: string = event.active_model || 'expert'; - for (const id in actionsWithFlags) { - const existing = (prevResultRef.current?.actions?.[id] || {}) as Partial; - actionsWithFlags[id] = { - ...actionsWithFlags[id], - is_manual: false, - origin: existing.origin ?? resultModel, - is_islanded: existing.is_islanded ?? actionsWithFlags[id].is_islanded, - estimated_max_rho: existing.estimated_max_rho ?? actionsWithFlags[id].max_rho, - estimated_max_rho_line: existing.estimated_max_rho_line ?? actionsWithFlags[id].max_rho_line, - }; - } - setSuggestedByRecommenderIds(prev => new Set([...prev, ...Object.keys(actionsWithFlags)])); - step2ActionsCount = Object.keys(actionsWithFlags).length; - const wallClockTime = (performance.now() - wallClockStart) / 1000; - setPendingAnalysisResult({ - ...event, - actions: actionsWithFlags, - wall_clock_time: wallClockTime, - }); - if (event.message) setInfoMessage(event.message); - } else if (event.type === 'error') { - setError('Analysis failed: ' + event.message); - } - } catch { - // Silent catch for incomplete rows + for await (const raw of parseNdjsonStream(response2)) { + const event = raw as Partial & { type?: string }; + if (event.type === 'pdf') { + setResult((p: AnalysisResult | null) => ({ + ...(p || {}), + pdf_url: event.pdf_url, + pdf_path: event.pdf_path, + // The overflow-graph build time is emitted on the + // ``pdf`` event (before discovery + assessment finish) + // so the iframe can display it below its title as soon + // as the file is ready. + ...(typeof event.overflow_graph_time === 'number' + ? { overflow_graph_time: event.overflow_graph_time } + : (event.overflow_graph_time === null + ? { overflow_graph_time: null } + : {})), + } as AnalysisResult)); + if (setActiveTab) { + setActiveTab('overflow'); } + } else if (event.type === 'result') { + const actionsWithFlags = { ...event.actions }; + // Provenance for recommender-produced actions: the model + // the backend actually ran (echoed as `active_model` on + // the result event). Preserve an existing `origin` so a + // "first guess" the operator added before analysis keeps + // its `"user"` provenance even when the model re-emits + // the same id. + const resultModel: string = event.active_model || 'expert'; + for (const id in actionsWithFlags) { + const existing = (prevResultRef.current?.actions?.[id] || {}) as Partial; + actionsWithFlags[id] = { + ...actionsWithFlags[id], + is_manual: false, + origin: existing.origin ?? resultModel, + is_islanded: existing.is_islanded ?? actionsWithFlags[id].is_islanded, + estimated_max_rho: existing.estimated_max_rho ?? actionsWithFlags[id].max_rho, + estimated_max_rho_line: existing.estimated_max_rho_line ?? actionsWithFlags[id].max_rho_line, + }; + } + setSuggestedByRecommenderIds(prev => new Set([...prev, ...Object.keys(actionsWithFlags)])); + step2ActionsCount = Object.keys(actionsWithFlags).length; + const wallClockTime = (performance.now() - wallClockStart) / 1000; + setPendingAnalysisResult({ + ...(event as AnalysisResult), + actions: actionsWithFlags, + wall_clock_time: wallClockTime, + }); + if (event.message) setInfoMessage(event.message); + } else if (event.type === 'error') { + setError('Analysis failed: ' + event.message); } } // Replay contract (docs/features/interaction-logging.md): diff --git a/frontend/src/hooks/useDiagrams.ts b/frontend/src/hooks/useDiagrams.ts index 449489f8..49458b68 100644 --- a/frontend/src/hooks/useDiagrams.ts +++ b/frontend/src/hooks/useDiagrams.ts @@ -14,6 +14,7 @@ import { } from '../utils/svgUtils'; import { processSvg } from '../utils/svgUtils'; import { cloneBaseSvg, applyPatchToClone } from '../utils/svgPatch'; +import { parseNdjsonStream } from '../utils/ndjsonStream'; import type { DiagramData, ViewBox, MetadataIndex, TabId, VlOverlay, SldTab, AnalysisResult, ActionDetail } from '../types'; import { interactionLogger } from '../utils/interactionLogger'; import { useSldOverlay } from './useSldOverlay'; @@ -631,68 +632,52 @@ export function useDiagrams( action_content: actionContent, lines_overloaded: linesOvl, }); - const reader = response.body!.getReader(); - const decoder = new TextDecoder(); - let buffer = ''; let streamErr: string | null = null; - while (true) { - const { value, done } = await reader.read(); - if (done) break; - buffer += decoder.decode(value, { stream: true }); - const lines = buffer.split('\n'); - buffer = lines.pop()!; - for (const line of lines) { - if (!line.trim()) continue; - let event: Record; - try { - event = JSON.parse(line); - } catch { - continue; // incomplete row - } - if (event.type === 'metrics') { - const simRes = event as unknown as { - description_unitaire: string; - rho_before: number[] | null; - rho_after: number[] | null; - max_rho: number | null; - max_rho_line: string; - is_rho_reduction: boolean; - non_convergence: string | null; - is_islanded?: boolean; - n_components?: number; - disconnected_mw?: number; + for await (const raw of parseNdjsonStream(response)) { + const event = raw as Record; + if (event.type === 'metrics') { + const simRes = event as unknown as { + description_unitaire: string; + rho_before: number[] | null; + rho_after: number[] | null; + max_rho: number | null; + max_rho_line: string; + is_rho_reduction: boolean; + non_convergence: string | null; + is_islanded?: boolean; + n_components?: number; + disconnected_mw?: number; + }; + setResult(prev => { + if (!prev) return prev; + const existing = prev.actions[actionId] || {} as Partial; + const hasRho = (existing.rho_before?.length ?? 0) > 0; + return { + ...prev, + actions: { + ...prev.actions, + [actionId]: { + ...existing, + description_unitaire: existing.description_unitaire || simRes.description_unitaire, + rho_before: hasRho ? existing.rho_before : simRes.rho_before, + rho_after: hasRho ? existing.rho_after : simRes.rho_after, + max_rho: hasRho ? existing.max_rho : simRes.max_rho, + max_rho_line: hasRho ? existing.max_rho_line : simRes.max_rho_line, + is_rho_reduction: hasRho ? existing.is_rho_reduction : simRes.is_rho_reduction, + non_convergence: simRes.non_convergence, + is_islanded: simRes.is_islanded, + n_components: simRes.n_components, + disconnected_mw: simRes.disconnected_mw, + } as ActionDetail, + }, }; - setResult(prev => { - if (!prev) return prev; - const existing = prev.actions[actionId] || {} as Partial; - const hasRho = (existing.rho_before?.length ?? 0) > 0; - return { - ...prev, - actions: { - ...prev.actions, - [actionId]: { - ...existing, - description_unitaire: existing.description_unitaire || simRes.description_unitaire, - rho_before: hasRho ? existing.rho_before : simRes.rho_before, - rho_after: hasRho ? existing.rho_after : simRes.rho_after, - max_rho: hasRho ? existing.max_rho : simRes.max_rho, - max_rho_line: hasRho ? existing.max_rho_line : simRes.max_rho_line, - is_rho_reduction: hasRho ? existing.is_rho_reduction : simRes.is_rho_reduction, - non_convergence: simRes.non_convergence, - is_islanded: simRes.is_islanded, - n_components: simRes.n_components, - disconnected_mw: simRes.disconnected_mw, - } as ActionDetail, - }, - }; - }); - } else if (event.type === 'diagram') { - const diag = event as unknown as DiagramData & { svg: string }; - const { svg, viewBox } = processSvg(diag.svg, voltageLevelsLength); - setActionDiagram({ ...diag, svg, originalViewBox: viewBox }); - } else if (event.type === 'error') { - streamErr = (event.message as string) || 'stream error'; - } + }); + } else if (event.type === 'diagram') { + const diag = event as unknown as DiagramData & { svg: string }; + const { svg, viewBox } = processSvg(diag.svg, voltageLevelsLength); + setActionDiagram({ ...diag, svg, originalViewBox: viewBox }); + } else if (event.type === 'error') { + streamErr = (event.message as string) || 'stream error'; } } if (streamErr) { diff --git a/frontend/src/utils/ndjsonStream.test.ts b/frontend/src/utils/ndjsonStream.test.ts new file mode 100644 index 00000000..f05b9197 --- /dev/null +++ b/frontend/src/utils/ndjsonStream.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +import { describe, it, expect } from 'vitest'; +import { parseNdjsonStream, type NdjsonSource } from './ndjsonStream'; + +/** Build a source whose body emits the given string chunks in order. */ +function sourceFromChunks(chunks: string[]): NdjsonSource { + const enc = new TextEncoder(); + let i = 0; + return { + body: new ReadableStream({ + pull(controller) { + if (i < chunks.length) controller.enqueue(enc.encode(chunks[i++])); + else controller.close(); + }, + }), + }; +} + +async function collect(src: NdjsonSource, opts?: { signal?: AbortSignal }): Promise { + const out: unknown[] = []; + for await (const ev of parseNdjsonStream(src, opts)) out.push(ev); + return out; +} + +describe('parseNdjsonStream', () => { + it('yields one parsed event per complete line', async () => { + const out = await collect(sourceFromChunks([ + JSON.stringify({ type: 'a' }) + '\n' + JSON.stringify({ type: 'b' }) + '\n', + ])); + expect(out).toEqual([{ type: 'a' }, { type: 'b' }]); + }); + + it('carries a partial line across chunk boundaries', async () => { + const line = JSON.stringify({ type: 'split', n: 42 }); + const mid = Math.floor(line.length / 2); + const out = await collect(sourceFromChunks([line.slice(0, mid), line.slice(mid) + '\n'])); + expect(out).toEqual([{ type: 'split', n: 42 }]); + }); + + it('splits a multi-line chunk emitted at once', async () => { + const out = await collect(sourceFromChunks([ + [JSON.stringify({ i: 1 }), JSON.stringify({ i: 2 }), JSON.stringify({ i: 3 }), ''].join('\n'), + ])); + expect(out).toEqual([{ i: 1 }, { i: 2 }, { i: 3 }]); + }); + + it('flushes a trailing line that lacks a final newline', async () => { + const out = await collect(sourceFromChunks([ + JSON.stringify({ type: 'a' }) + '\n' + JSON.stringify({ type: 'last' }), + ])); + expect(out).toEqual([{ type: 'a' }, { type: 'last' }]); + }); + + it('skips blank lines', async () => { + const out = await collect(sourceFromChunks(['\n\n' + JSON.stringify({ type: 'a' }) + '\n\n'])); + expect(out).toEqual([{ type: 'a' }]); + }); + + it('skips a complete line that fails JSON.parse', async () => { + const out = await collect(sourceFromChunks([ + 'not json\n' + JSON.stringify({ type: 'ok' }) + '\n', + ])); + expect(out).toEqual([{ type: 'ok' }]); + }); + + it('does not yield a malformed trailing line', async () => { + const out = await collect(sourceFromChunks([JSON.stringify({ type: 'a' }) + '\n{ partial'])); + expect(out).toEqual([{ type: 'a' }]); + }); + + it('returns nothing for a null body', async () => { + const out = await collect({ body: null }); + expect(out).toEqual([]); + }); + + it('returns nothing when the signal is already aborted', async () => { + const ctrl = new AbortController(); + ctrl.abort(); + const out = await collect( + sourceFromChunks([JSON.stringify({ type: 'a' }) + '\n']), + { signal: ctrl.signal }, + ); + expect(out).toEqual([]); + }); + + it('stops without flushing the partial buffer when aborted mid-stream', async () => { + const enc = new TextEncoder(); + const ctrl = new AbortController(); + let pulled = 0; + const src: NdjsonSource = { + body: new ReadableStream({ + pull(controller) { + pulled++; + if (pulled === 1) { + // One complete line + a VALID but unterminated trailing line. + controller.enqueue(enc.encode( + JSON.stringify({ type: 'first' }) + '\n' + JSON.stringify({ type: 'trailing' }), + )); + } else { + // Abort before any more data / close arrives. + ctrl.abort(); + } + }, + }), + }; + const out = await collect(src, { signal: ctrl.signal }); + // The completed line is delivered; the unterminated trailing line is + // NOT flushed because the stream was aborted, not finished. + expect(out).toEqual([{ type: 'first' }]); + }); +}); diff --git a/frontend/src/utils/ndjsonStream.ts b/frontend/src/utils/ndjsonStream.ts new file mode 100644 index 00000000..37419cff --- /dev/null +++ b/frontend/src/utils/ndjsonStream.ts @@ -0,0 +1,107 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +// Single reader for the backend's NDJSON (newline-delimited JSON) streams +// (`/api/run-analysis[-step2]`, `/api/simulate-and-variant-diagram`). This +// replaces five hand-copied reader loops (App.tsx ×2, useAnalysis, +// useDiagrams, ActionFeed) that had drifted apart: only the two App.tsx +// copies flushed a final line that lacked a trailing newline, and each +// re-implemented blank-line / parse-error handling slightly differently +// (D5 / QW10, 2026-07). +// +// Uniform semantics for every caller: +// - decodes with `TextDecoder({stream:true})`, carrying the partial line +// across chunk boundaries; +// - yields one parsed value per COMPLETE line; blank lines are skipped; +// - a line that fails `JSON.parse` is skipped (matches the legacy +// "silent catch for incomplete rows" behaviour); +// - on stream end, flushes the decoder and yields a trailing unterminated +// final line if present (the robustness fix only two copies carried); +// - honours an optional `AbortSignal`: aborting cancels the reader and +// ends the iteration cleanly (a `for await` consumer simply stops and +// the partial buffer is NOT flushed). Consumers distinguish an abort +// from normal completion by checking `signal.aborted` after the loop. + +/** Anything with a readable body — a `fetch` `Response`, or a test double. */ +export interface NdjsonSource { + body: ReadableStream | null; +} + +export interface NdjsonStreamOptions { + /** Abort the read (and end the iteration) when this signal fires. */ + signal?: AbortSignal; +} + +function parseLine(line: string): { ok: true; value: unknown } | { ok: false } { + if (!line.trim()) return { ok: false }; + try { + return { ok: true, value: JSON.parse(line) }; + } catch { + // A complete line that won't parse is malformed/partial — skip it, + // preserving the legacy loops' silent behaviour rather than throwing + // into the middle of a caller's event switch. + return { ok: false }; + } +} + +/** + * Parse an NDJSON HTTP streaming body into an async iterable of parsed + * events. Cast each yielded value to the caller's event union. + */ +export async function* parseNdjsonStream( + source: NdjsonSource, + options: NdjsonStreamOptions = {}, +): AsyncGenerator { + const { signal } = options; + const body = source.body; + if (!body) return; + if (signal?.aborted) return; + + const reader = body.getReader(); + const decoder = new TextDecoder(); + let buffer = ''; + + // A pending `reader.read()` won't settle until the next chunk arrives; + // cancelling the reader on abort forces it to resolve so the loop can + // notice the abort promptly even when the fetch itself wasn't wired to + // the signal. + // `reader.cancel?.()?.catch(...)` tolerates a reader that lacks + // `cancel` (some test doubles) and never throws out of cleanup. + const cancelReader = () => { reader.cancel?.()?.catch(() => { /* already closed */ }); }; + const onAbort = () => { cancelReader(); }; + if (signal) signal.addEventListener('abort', onAbort); + + try { + while (true) { + let chunk: ReadableStreamReadResult; + try { + chunk = await reader.read(); + } catch (err) { + if (signal?.aborted) return; // aborted mid-read → end cleanly + throw err; + } + if (chunk.done) break; + buffer += decoder.decode(chunk.value, { stream: true }); + const lines = buffer.split('\n'); + buffer = lines.pop() ?? ''; + for (const line of lines) { + const parsed = parseLine(line); + if (parsed.ok) yield parsed.value; + } + } + // Don't flush a partial buffer that an abort left behind. + if (signal?.aborted) return; + // Flush the decoder's held bytes, then any trailing unterminated line. + buffer += decoder.decode(); + const parsed = parseLine(buffer); + if (parsed.ok) yield parsed.value; + } finally { + if (signal) signal.removeEventListener('abort', onAbort); + // Release/cancel the underlying stream — important when a consumer + // `break`s out early (e.g. on abort) so the network body is freed. + cancelReader(); + } +} From 8027a7ffcd5d54b3afe63fc96ac09ae3fa15a28a Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Wed, 8 Jul 2026 20:50:12 +0000 Subject: [PATCH 04/24] feat(D5.2): typed notification store replaces the dual toast channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the two ad-hoc toast channels — a sticky-but-undismissable `error` string and an auto-3s `infoMessage` string whose green-vs-blue colour was decided by a magic `'SUCCESS'` prefix — with one typed store. - utils/notifications.ts: a module-singleton store (severity error/ success/info, sticky flag, per-item auto-expiry, de-dupe of identical toasts) bound to React via useSyncExternalStore (`useNotifications`), with notifyError/notifyInfo/notifySuccess/dismiss/clear helpers. - components/NotificationHost.tsx: renders the stack bottom-right, each toast dismissible, inside an aria-live region (errors role=alert / assertive, info+success role=status / polite). Replaces StatusToasts (deleted). - useAnalysis: drops its local error/infoMessage state + bespoke 3s timer; setError/setInfoMessage are now thin adapters over the store (same `(v)=>void` raise/clear contract). - useSession: the two `setInfoMessage('SUCCESS: …')` sites now call notifySuccess directly, retiring the string protocol; the now-unused setInfoMessage param is dropped from SaveParams / RestoreContext. - App: setError is a store adapter (all call sites + hook threading unchanged); renders NotificationHost; drops the infoMessage plumbing. Tests: notifications.test.ts + NotificationHost.test.tsx added; useAnalysis.test asserts against the store; setup.ts clears the singleton between tests. Full Vitest suite green (1753 passed). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- frontend/src/App.tsx | 31 ++-- .../src/components/NotificationHost.test.tsx | 44 +++++ frontend/src/components/NotificationHost.tsx | 93 +++++++++++ frontend/src/components/StatusToasts.tsx | 49 ------ frontend/src/hooks/useAnalysis.test.ts | 25 ++- frontend/src/hooks/useAnalysis.ts | 36 ++-- frontend/src/hooks/useSession.ts | 7 +- frontend/src/test/setup.ts | 6 + frontend/src/utils/notifications.test.ts | 109 ++++++++++++ frontend/src/utils/notifications.ts | 157 ++++++++++++++++++ 10 files changed, 469 insertions(+), 88 deletions(-) create mode 100644 frontend/src/components/NotificationHost.test.tsx create mode 100644 frontend/src/components/NotificationHost.tsx delete mode 100644 frontend/src/components/StatusToasts.tsx create mode 100644 frontend/src/utils/notifications.test.ts create mode 100644 frontend/src/utils/notifications.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 13b620a7..aefc3d30 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -12,7 +12,7 @@ import ActionFeed from './components/ActionFeed'; import AntennaNotice from './components/AntennaNotice'; import Header from './components/Header'; import AppSidebar from './components/AppSidebar'; -import StatusToasts from './components/StatusToasts'; +import NotificationHost from './components/NotificationHost'; import type { Notice } from './components/NoticesPanel'; import SettingsModal from './components/modals/SettingsModal'; import ReloadSessionModal from './components/modals/ReloadSessionModal'; @@ -36,6 +36,7 @@ import type { GameStudy } from './game/types'; import { DEFAULT_ACTION_OVERVIEW_FILTERS } from './utils/actionTypes'; import { apiErrorMessage } from './utils/apiError'; import { parseNdjsonStream } from './utils/ndjsonStream'; +import { notifyError, notifications } from './utils/notifications'; import { attachVlInteractions } from './utils/svgUtils'; import { buildOverflowPinPayload, @@ -102,7 +103,13 @@ function App() { * a non-empty `result.actions` map. */ const [overflowPinsEnabled, setOverflowPinsEnabled] = useState(false); const [configLoading, setConfigLoading] = useState(false); - const [error, setError] = useState(''); + // Errors surface through the shared notification store (D5). This + // adapter preserves the historical `setError(msg)` / `setError('')` + // (raise / clear) contract used across App and threaded into hooks. + const setError = useCallback((message: string) => { + if (message) notifyError(message); + else notifications.clearSeverity('error'); + }, []); /** Resolve an element or VL ID to its display name. Falls back to the ID. */ const displayName = useCallback((id: string) => nameMap[id] || id, [nameMap]); @@ -189,7 +196,7 @@ function App() { const analysis = useAnalysis(); const { result, setResult, pendingAnalysisResult, analysisLoading, - infoMessage, selectedOverloads, monitorDeselected, + selectedOverloads, monitorDeselected, additionalLinesToCut, committedAdditionalLinesToCut, } = analysis; @@ -328,11 +335,9 @@ function App() { // Fresh contingency / study starts in hierarchical mode so the // toggle matches the backend's freshly-cleared overflow cache. diagrams.setOverflowLayoutMode('hierarchical'); + // Clear both notification channels so a failed-analysis error or a + // stale info toast doesn't outlive the contingency it described. setError(''); - // The analysis hook owns its own `error` (surfaced via the shared - // StatusToasts channel — see the render site). Clear it here too so - // a failed-analysis banner doesn't survive a contingency clear. - analysis.setError(''); analysis.setInfoMessage(''); diagrams.setInspectQuery(''); // Do NOT reset lastZoomState here. Resetting it causes the auto-zoom @@ -640,7 +645,7 @@ function App() { setError(apiErrorMessage(err, 'Simulation failed')); } }, - [selectedContingency, result?.lines_overloaded, result?.active_model, diagrams, voltageLevels.length, wrappedManualActionAdded] + [selectedContingency, result?.lines_overloaded, result?.active_model, diagrams, voltageLevels.length, wrappedManualActionAdded, setError] ); // Interactive SLD topology edit (useSldTopologyEdit). The baseline @@ -840,7 +845,7 @@ function App() { }, [ diagrams, sldTopologyEdit, selectedContingency, sldEditBaseActionId, result?.lines_overloaded, voltageLevels.length, wrappedManualActionAdded, - handleVlDoubleClick, + handleVlDoubleClick, setError, ]); // Re-simulation of an already-present action (edit Target MW / tap on a @@ -976,7 +981,7 @@ function App() { n1OverloadsRho: n1Diagram?.lines_overloaded_rho, result, selectedActionIds, rejectedActionIds, manuallyAddedIds, suggestedByRecommenderIds, - setError, setInfoMessage: analysis.setInfoMessage, + setError, }), [ networkPath, actionPath, layoutPath, outputFolderPath, minLineReconnections, minCloseCoupling, minOpenCoupling, @@ -989,7 +994,7 @@ function App() { nDiagram, n1Diagram, result, selectedActionIds, rejectedActionIds, manuallyAddedIds, suggestedByRecommenderIds, - setError, analysis.setInfoMessage, + setError, ]); const wrappedSaveResults = useCallback( @@ -1024,7 +1029,7 @@ function App() { restoringSessionRef: diagrams.restoringSessionRef, committedBranchRef: diagrams.committedBranchRef, committedNetworkPathRef, - setError, setInfoMessage: analysis.setInfoMessage, + setError, applyConfigResponse, setBranches, setVoltageLevels, setNameMap, setNominalVoltageMap: diagrams.setNominalVoltageMap, setUniqueVoltages: diagrams.setUniqueVoltages, @@ -2034,7 +2039,7 @@ function App() { onCancel={handleCancelDialog} onConfirm={handleConfirmDialog} /> - + ); } diff --git a/frontend/src/components/NotificationHost.test.tsx b/frontend/src/components/NotificationHost.test.tsx new file mode 100644 index 00000000..a840a337 --- /dev/null +++ b/frontend/src/components/NotificationHost.test.tsx @@ -0,0 +1,44 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +import { describe, it, expect, beforeEach } from 'vitest'; +import { render, screen, act, cleanup } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import NotificationHost from './NotificationHost'; +import { notifications, notifyError, notifyInfo, notifySuccess } from '../utils/notifications'; + +describe('NotificationHost', () => { + beforeEach(() => { notifications.clear(); cleanup(); }); + + it('renders nothing when there are no notifications', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders a toast per notification with severity-tagged testids', () => { + render(); + act(() => { notifyError('an error'); notifyInfo('some info'); notifySuccess('done'); }); + expect(screen.getByTestId('notification-error')).toHaveTextContent('an error'); + expect(screen.getByTestId('notification-info')).toHaveTextContent('some info'); + expect(screen.getByTestId('notification-success')).toHaveTextContent('done'); + }); + + it('announces errors assertively (role=alert) and others politely (role=status)', () => { + render(); + act(() => { notifyError('boom'); notifyInfo('fyi'); }); + expect(screen.getByTestId('notification-error')).toHaveAttribute('role', 'alert'); + expect(screen.getByTestId('notification-info')).toHaveAttribute('role', 'status'); + }); + + it('dismisses a toast when its dismiss button is clicked', async () => { + const user = userEvent.setup(); + render(); + act(() => { notifyError('go away'); }); + expect(screen.getByText('go away')).toBeInTheDocument(); + await user.click(screen.getByRole('button', { name: /dismiss notification/i })); + expect(screen.queryByText('go away')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/NotificationHost.tsx b/frontend/src/components/NotificationHost.tsx new file mode 100644 index 00000000..11543370 --- /dev/null +++ b/frontend/src/components/NotificationHost.tsx @@ -0,0 +1,93 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +import { colors, radius, space } from '../styles/tokens'; +import { + useNotifications, + dismissNotification, + type Notification, + type NotificationSeverity, +} from '../utils/notifications'; + +/** + * Renders the typed notification store (D5) as a stack of dismissible + * toasts pinned bottom-right. Replaces the former `StatusToasts` dual + * error/info banners: severity is explicit (no `'SUCCESS'` string + * protocol), every toast has a dismiss control, and the stack lives in an + * `aria-live` region so assistive tech announces new toasts (errors + * assertively via `role="alert"`, info/success politely). + */ + +const SEVERITY_BG: Record = { + error: colors.danger, + success: colors.success, + info: colors.brand, +}; + +function Toast({ item }: { item: Notification }) { + return ( +
+ {item.message} + +
+ ); +} + +export default function NotificationHost() { + const items = useNotifications(); + if (items.length === 0) return null; + return ( +
+ {items.map(item => )} +
+ ); +} diff --git a/frontend/src/components/StatusToasts.tsx b/frontend/src/components/StatusToasts.tsx deleted file mode 100644 index 71db2220..00000000 --- a/frontend/src/components/StatusToasts.tsx +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) -// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. -// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, -// you can obtain one at http://mozilla.org/MPL/2.0/. -// SPDX-License-Identifier: MPL-2.0 -// This file is part of Co-Study4Grid a Power Grid Study tool Assistant Interface to help solve contigencies for a grid state under study. - -import { colors, radius, space } from '../styles/tokens'; - -interface StatusToastsProps { - error: string; - infoMessage: string; -} - -/** - * Fixed-position status banners shown in the bottom corners of the - * window. The error banner is red and pinned bottom-right; the info - * banner is green (on SUCCESS-prefixed messages) or blue and pinned - * bottom-left. - */ -export default function StatusToasts({ error, infoMessage }: StatusToastsProps) { - return ( - <> - {error && ( -
- {error} -
- )} - {infoMessage && ( -
- {infoMessage} -
- )} - - ); -} diff --git a/frontend/src/hooks/useAnalysis.test.ts b/frontend/src/hooks/useAnalysis.test.ts index f55a5b6e..867c4cce 100644 --- a/frontend/src/hooks/useAnalysis.test.ts +++ b/frontend/src/hooks/useAnalysis.test.ts @@ -9,6 +9,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { renderHook, act } from '@testing-library/react'; import { useAnalysis } from './useAnalysis'; import { interactionLogger } from '../utils/interactionLogger'; +import { notifications, DEFAULT_TIMEOUT_MS, type NotificationSeverity } from '../utils/notifications'; + +/** Messages currently in the notification store for a given severity. */ +function toastMessages(severity: NotificationSeverity): string[] { + return notifications.getSnapshot().filter(n => n.severity === severity).map(n => n.message); +} // Mock the api module (dynamic import in useAnalysis) const mockRunAnalysisStep1 = vi.fn(); @@ -36,13 +42,14 @@ describe('useAnalysis', () => { beforeEach(() => { vi.clearAllMocks(); interactionLogger.clear(); + notifications.clear(); }); it('initializes with null result and no loading', () => { const { result } = renderHook(() => useAnalysis()); expect(result.current.result).toBeNull(); expect(result.current.analysisLoading).toBe(false); - expect(result.current.error).toBe(''); + expect(notifications.getSnapshot()).toHaveLength(0); }); it('does nothing when selectedBranch is empty', async () => { @@ -73,7 +80,7 @@ describe('useAnalysis', () => { await result.current.handleRunAnalysis(['LINE_X'], clear, setSuggested); }); - expect(result.current.error).toBe('Network not loaded'); + expect(toastMessages('error')).toContain('Network not loaded'); expect(result.current.analysisLoading).toBe(false); expect(mockRunAnalysisStep2Stream).not.toHaveBeenCalled(); }); @@ -91,7 +98,7 @@ describe('useAnalysis', () => { await result.current.handleRunAnalysis(['LINE_X'], vi.fn(), vi.fn()); }); - expect(result.current.infoMessage).toBe('No overloads detected'); + expect(toastMessages('info')).toContain('No overloads detected'); expect(result.current.analysisLoading).toBe(false); }); @@ -307,7 +314,7 @@ describe('useAnalysis', () => { await result.current.handleRunAnalysis(['LINE_X'], vi.fn(), vi.fn()); }); - expect(result.current.error).toBe('Analysis failed: Backend crashed'); + expect(toastMessages('error')).toContain('Analysis failed: Backend crashed'); }); it('sets error when step2 rejects', async () => { @@ -322,7 +329,7 @@ describe('useAnalysis', () => { await result.current.handleRunAnalysis(['LINE_X'], vi.fn(), vi.fn()); }); - expect(result.current.error).toBe('Network error'); + expect(toastMessages('error')).toContain('Network error'); expect(result.current.analysisLoading).toBe(false); }); @@ -576,15 +583,15 @@ describe('useAnalysis', () => { }); describe('info message auto-clear', () => { - it('clears info message after timeout', async () => { + it('raises an info toast that the store auto-dismisses after its timeout', () => { vi.useFakeTimers(); const { result } = renderHook(() => useAnalysis()); act(() => { result.current.setInfoMessage('Temporary message'); }); - expect(result.current.infoMessage).toBe('Temporary message'); + expect(toastMessages('info')).toContain('Temporary message'); - act(() => { vi.advanceTimersByTime(3000); }); - expect(result.current.infoMessage).toBe(''); + act(() => { vi.advanceTimersByTime(DEFAULT_TIMEOUT_MS); }); + expect(toastMessages('info')).not.toContain('Temporary message'); vi.useRealTimers(); }); diff --git a/frontend/src/hooks/useAnalysis.ts b/frontend/src/hooks/useAnalysis.ts index c57f8978..6cd94637 100644 --- a/frontend/src/hooks/useAnalysis.ts +++ b/frontend/src/hooks/useAnalysis.ts @@ -9,6 +9,7 @@ import { useState, useEffect, useCallback, useMemo, useRef, type Dispatch, type import type { ActionDetail, AnalysisResult, TabId } from '../types'; import { interactionLogger } from '../utils/interactionLogger'; import { parseNdjsonStream } from '../utils/ndjsonStream'; +import { notifications, notifyError, notifyInfo } from '../utils/notifications'; export interface AnalysisState { result: AnalysisResult | null; @@ -17,9 +18,9 @@ export interface AnalysisState { setPendingAnalysisResult: (v: AnalysisResult | null) => void; analysisLoading: boolean; setAnalysisLoading: (v: boolean) => void; - infoMessage: string; + /** Raise an auto-dismissing info toast (`''` clears the info channel). */ setInfoMessage: (v: string) => void; - error: string; + /** Raise a sticky error toast (`''` clears the error channel). */ setError: (v: string) => void; // Analysis flow @@ -69,15 +70,24 @@ export function useAnalysis(): AnalysisState { const [pendingAnalysisResult, setPendingAnalysisResult] = useState(null); const [analysisLoading, setAnalysisLoading] = useState(false); - const [infoMessage, setInfoMessage] = useState(''); - const [error, setError] = useState(''); - useEffect(() => { - if (infoMessage) { - const timer = setTimeout(() => { setInfoMessage(''); }, 3000); - return () => clearTimeout(timer); + // Errors / info route through the shared notification store (D5) rather + // than local `error` / `infoMessage` string state with a bespoke 3s + // timer. These adapters keep the historical `(v: string) => void` + // call-site contract: a message raises a toast; an empty string clears + // that channel (the "reset before a run" gesture). + const setError = useCallback((message: string) => { + if (message) notifyError(message); + else notifications.clearSeverity('error'); + }, []); + const setInfoMessage = useCallback((message: string) => { + if (message) { + notifyInfo(message); + } else { + notifications.clearSeverity('info'); + notifications.clearSeverity('success'); } - }, [infoMessage]); + }, []); // Analysis flow const [selectedOverloads, setSelectedOverloads] = useState>(new Set()); @@ -243,7 +253,7 @@ export function useAnalysis(): AnalysisState { } finally { setAnalysisLoading(false); } - }, [selectedOverloads, monitorDeselected, additionalLinesToCut]); + }, [selectedOverloads, monitorDeselected, additionalLinesToCut, setError, setInfoMessage]); const handleToggleAdditionalLineToCut = useCallback((line: string) => { if (!line) return; @@ -335,8 +345,8 @@ export function useAnalysis(): AnalysisState { result, setResult, pendingAnalysisResult, setPendingAnalysisResult, analysisLoading, setAnalysisLoading, - infoMessage, setInfoMessage, - error, setError, + setInfoMessage, + setError, selectedOverloads, setSelectedOverloads, monitorDeselected, setMonitorDeselected, additionalLinesToCut, setAdditionalLinesToCut, @@ -347,7 +357,7 @@ export function useAnalysis(): AnalysisState { handleDisplayPrioritizedActions, handleToggleOverload, }), [ - result, pendingAnalysisResult, analysisLoading, infoMessage, error, + result, pendingAnalysisResult, analysisLoading, setInfoMessage, setError, selectedOverloads, monitorDeselected, additionalLinesToCut, committedAdditionalLinesToCut, handleRunAnalysis, handleDisplayPrioritizedActions, handleToggleOverload, diff --git a/frontend/src/hooks/useSession.ts b/frontend/src/hooks/useSession.ts index e0672a40..af14121b 100644 --- a/frontend/src/hooks/useSession.ts +++ b/frontend/src/hooks/useSession.ts @@ -10,6 +10,7 @@ import { api } from '../api'; import { buildSessionResult } from '../utils/sessionUtils'; import { interactionLogger } from '../utils/interactionLogger'; import { apiErrorMessage } from '../utils/apiError'; +import { notifySuccess } from '../utils/notifications'; import type { AnalysisResult, CombinedAction, ActionDetail, SessionResult, DiagramData } from '../types'; export interface SessionState { @@ -67,7 +68,6 @@ export interface SaveParams { rejectedActionIds: Set; manuallyAddedIds: Set; suggestedByRecommenderIds: Set; - setInfoMessage: (v: string) => void; setError: (v: string) => void; } @@ -137,7 +137,6 @@ export interface RestoreContext { * ``committedBranchRef`` while the restore is still in flight. * Optional for test mocks that predate the field. */ resetAllState?: () => void; - setInfoMessage: (v: string) => void; setError: (v: string) => void; } @@ -203,7 +202,7 @@ export function useSession(): SessionState { interaction_log: JSON.stringify(interactionLogger.getLog(), null, 2), }); const pdfMsg = res.pdf_copied ? " (including PDF)" : " (PDF not found)"; - params.setInfoMessage(`SUCCESS: Session saved to: ${res.session_folder}${pdfMsg}`); + notifySuccess(`Session saved to: ${res.session_folder}${pdfMsg}`); } catch (err: unknown) { const e = err as { response?: { data?: { detail?: string } }; message?: string }; params.setError('Failed to save session: ' + apiErrorMessage(e)); @@ -510,7 +509,7 @@ export function useSession(): SessionState { setShowReloadModal(false); interactionLogger.record('session_reloaded', { session_name: sessionName }); - ctx.setInfoMessage(`SUCCESS: Session "${sessionName}" restored`); + notifySuccess(`Session "${sessionName}" restored`); } catch (err: unknown) { // If the restore aborts before useContingencyFetch had a chance // to consume the flag, clear it here — leaving it sticky would diff --git a/frontend/src/test/setup.ts b/frontend/src/test/setup.ts index 5b8d9e48..e813c5b4 100644 --- a/frontend/src/test/setup.ts +++ b/frontend/src/test/setup.ts @@ -6,10 +6,16 @@ // This file is part of Co-Study4Grid a Power Grid Study tool Assistant Interface to help solve contigencies for a grid state under study. import '@testing-library/jest-dom/vitest'; +import { afterEach } from 'vitest'; +import { notifications } from '../utils/notifications'; // @ts-expect-error: IS_REACT_ACT_ENVIRONMENT is not in the global type definitions global.IS_REACT_ACT_ENVIRONMENT = true; +// The notification store (D5) is a module singleton, so reset it between +// tests to keep toasts from one test leaking into the next. +afterEach(() => { notifications.clear(); }); + // Mock SVG getScreenCTM and related methods which are not implemented in JSDOM if (typeof window !== 'undefined') { const mockMatrix = { diff --git a/frontend/src/utils/notifications.test.ts b/frontend/src/utils/notifications.test.ts new file mode 100644 index 00000000..eeb2a275 --- /dev/null +++ b/frontend/src/utils/notifications.test.ts @@ -0,0 +1,109 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + notifications, + notifyError, + notifyInfo, + notifySuccess, + dismissNotification, + clearNotifications, + DEFAULT_TIMEOUT_MS, +} from './notifications'; + +const snap = () => notifications.getSnapshot(); + +describe('notification store', () => { + beforeEach(() => { notifications.clear(); }); + + it('raises a notification with the given severity + message', () => { + notifyError('boom'); + expect(snap()).toHaveLength(1); + expect(snap()[0]).toMatchObject({ severity: 'error', message: 'boom' }); + }); + + it('trims and ignores empty messages', () => { + expect(notifyInfo(' ')).toBe(-1); + expect(snap()).toHaveLength(0); + }); + + it('makes errors sticky and info/success non-sticky by default', () => { + notifyError('e'); + notifyInfo('i'); + notifySuccess('s'); + const bySeverity = Object.fromEntries(snap().map(n => [n.severity, n.sticky])); + expect(bySeverity).toEqual({ error: true, info: false, success: false }); + }); + + it('de-dupes an identical severity+message instead of stacking', () => { + notifyError('same'); + notifyError('same'); + expect(snap()).toHaveLength(1); + // A different message, or a different severity, is a separate toast. + notifyError('other'); + notifyInfo('same'); + expect(snap()).toHaveLength(3); + }); + + it('dismisses a single notification by id', () => { + const id = notifyError('x'); + notifyError('y'); + dismissNotification(id); + expect(snap().map(n => n.message)).toEqual(['y']); + }); + + it('clears a whole severity', () => { + notifyError('e1'); + notifyError('e2'); + notifyInfo('keep'); + notifications.clearSeverity('error'); + expect(snap().map(n => n.message)).toEqual(['keep']); + }); + + it('clears everything', () => { + notifyError('e'); + notifyInfo('i'); + clearNotifications(); + expect(snap()).toHaveLength(0); + }); + + it('notifies subscribers on change', () => { + const listener = vi.fn(); + const unsub = notifications.subscribe(listener); + notifyError('a'); + expect(listener).toHaveBeenCalledTimes(1); + unsub(); + notifyError('b'); + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('returns a stable snapshot reference between mutations', () => { + notifyError('a'); + const first = snap(); + expect(snap()).toBe(first); // no mutation → same array reference + notifyInfo('b'); + expect(snap()).not.toBe(first); // mutation → new reference + }); + + describe('auto-expiry', () => { + beforeEach(() => { vi.useFakeTimers(); }); + afterEach(() => { vi.useRealTimers(); }); + + it('auto-dismisses non-sticky toasts after the timeout', () => { + notifyInfo('temp'); + expect(snap()).toHaveLength(1); + vi.advanceTimersByTime(DEFAULT_TIMEOUT_MS); + expect(snap()).toHaveLength(0); + }); + + it('keeps sticky (error) toasts past the timeout', () => { + notifyError('stays'); + vi.advanceTimersByTime(DEFAULT_TIMEOUT_MS * 3); + expect(snap()).toHaveLength(1); + }); + }); +}); diff --git a/frontend/src/utils/notifications.ts b/frontend/src/utils/notifications.ts new file mode 100644 index 00000000..5e83d886 --- /dev/null +++ b/frontend/src/utils/notifications.ts @@ -0,0 +1,157 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +// Typed notification store (D5, 2026-07). Replaces the two ad-hoc toast +// channels — a sticky-but-undismissable `error` string and an +// auto-3s-hiding `infoMessage` string whose green-vs-blue colour was +// decided by a magic `'SUCCESS'` prefix — with ONE store carrying an +// explicit severity, a sticky flag, per-item dismissal and (via +// NotificationHost) an aria-live region. +// +// It is a module singleton (mirroring `interactionLogger`) so any layer — +// App, hooks, even non-React helpers — can raise a notification without +// prop-threading a setter, and `useNotifications()` subscribes React to it +// through `useSyncExternalStore`. + +import { useSyncExternalStore } from 'react'; + +export type NotificationSeverity = 'error' | 'success' | 'info'; + +export interface Notification { + /** Monotonic id, stable for the item's lifetime (used as React key). */ + readonly id: number; + readonly severity: NotificationSeverity; + readonly message: string; + /** Sticky items stay until dismissed; others auto-expire. */ + readonly sticky: boolean; +} + +export interface NotifyOptions { + /** Override the severity default (errors sticky, info/success not). */ + sticky?: boolean; + /** Auto-dismiss delay for non-sticky items. Defaults to + * `DEFAULT_TIMEOUT_MS`. Ignored when sticky. */ + timeoutMs?: number; +} + +/** Non-sticky items auto-dismiss after this long by default. */ +export const DEFAULT_TIMEOUT_MS = 4000; + +type Timer = ReturnType; + +class NotificationStore { + private items: readonly Notification[] = []; + private readonly listeners = new Set<() => void>(); + private readonly timers = new Map(); + private seq = 0; + + subscribe = (listener: () => void): (() => void) => { + this.listeners.add(listener); + return () => { this.listeners.delete(listener); }; + }; + + /** Stable reference between mutations — required by useSyncExternalStore. */ + getSnapshot = (): readonly Notification[] => this.items; + + private emit(): void { + for (const listener of this.listeners) listener(); + } + + private clearTimer(id: number): void { + const t = this.timers.get(id); + if (t !== undefined) { + clearTimeout(t); + this.timers.delete(id); + } + } + + notify(severity: NotificationSeverity, message: string, options: NotifyOptions = {}): number { + const text = (message ?? '').trim(); + if (!text) return -1; + const sticky = options.sticky ?? severity === 'error'; + + // De-dupe: an identical (severity, message) already showing is + // refreshed rather than stacked — stops a debounced/retrying caller + // (e.g. the SLD preview) from piling up copies of one error. + const existing = this.items.find(n => n.severity === severity && n.message === text); + if (existing) { + if (!sticky) this.scheduleExpiry(existing.id, options.timeoutMs); + return existing.id; + } + + const id = ++this.seq; + this.items = [...this.items, { id, severity, message: text, sticky }]; + if (!sticky) this.scheduleExpiry(id, options.timeoutMs); + this.emit(); + return id; + } + + private scheduleExpiry(id: number, timeoutMs?: number): void { + this.clearTimer(id); + this.timers.set(id, setTimeout(() => this.dismiss(id), timeoutMs ?? DEFAULT_TIMEOUT_MS)); + } + + dismiss(id: number): void { + this.clearTimer(id); + const next = this.items.filter(n => n.id !== id); + if (next.length !== this.items.length) { + this.items = next; + this.emit(); + } + } + + /** Remove every notification of a severity (the "clear the error" gesture). */ + clearSeverity(severity: NotificationSeverity): void { + const next = this.items.filter(n => n.severity !== severity); + if (next.length !== this.items.length) { + for (const n of this.items) if (n.severity === severity) this.clearTimer(n.id); + this.items = next; + this.emit(); + } + } + + /** Remove everything (e.g. on a study reset). */ + clear(): void { + if (this.items.length === 0) return; + for (const id of this.timers.keys()) this.clearTimer(id); + this.items = []; + this.emit(); + } +} + +export const notifications = new NotificationStore(); + +// --- Convenience raisers ------------------------------------------------- + +/** Raise a sticky error toast (stays until dismissed). */ +export function notifyError(message: string): number { + return notifications.notify('error', message); +} + +/** Raise an auto-dismissing info toast. */ +export function notifyInfo(message: string): number { + return notifications.notify('info', message); +} + +/** Raise an auto-dismissing success toast (replaces the `'SUCCESS:'` prefix). */ +export function notifySuccess(message: string): number { + return notifications.notify('success', message); +} + +export function dismissNotification(id: number): void { + notifications.dismiss(id); +} + +export function clearNotifications(): void { + notifications.clear(); +} + +// --- React binding ------------------------------------------------------- + +/** Subscribe a component to the live notification list. */ +export function useNotifications(): readonly Notification[] { + return useSyncExternalStore(notifications.subscribe, notifications.getSnapshot, notifications.getSnapshot); +} From 90d867c3e5bfcfe669f1517b0ed84007529ebe14 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Wed, 8 Jul 2026 20:59:29 +0000 Subject: [PATCH 05/24] feat(D5.3): cancellable analysis via AbortController + visible Cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long-running analysis runs are now cancellable, completing the D5 UX fix for T4 (error/streaming paths as UX). - api.ts: runAnalysisStep1 / runAnalysisStep2Stream accept an optional AbortSignal (axios `signal` / fetch `signal`). - useAnalysis: each run gets a fresh AbortController (kept on a ref); its signal is threaded into step 1, the step 2 fetch, and the NDJSON stream read. `cancelAnalysis()` aborts it. An abort is treated as a cancellation (info toast "Analysis cancelled.", no error toast, no step-2 completion event), not a failure. - ActionFeed: a "✕ Cancel" control appears beside "⚙️ Analyzing…" while a run is in flight (guarded by an optional onCancelAnalysis prop); App wires it to analysis.cancelAnalysis. Tests: hook-level cancellation (signal threading, abort → cancellation notice not error, no-op when idle), the ActionFeed Cancel control, and the existing step2 call assertions updated for the new signal arg. Full Vitest suite green (1758 passed). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- frontend/src/App.tsx | 1 + frontend/src/api.test.ts | 1 + frontend/src/api.ts | 7 +-- frontend/src/components/ActionFeed.test.tsx | 14 ++++++ frontend/src/components/ActionFeed.tsx | 38 ++++++++++++---- frontend/src/hooks/useAnalysis.test.ts | 50 ++++++++++++++++++++- frontend/src/hooks/useAnalysis.ts | 41 ++++++++++++++--- 7 files changed, 133 insertions(+), 19 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index aefc3d30..51839245 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1894,6 +1894,7 @@ function App() { pendingAnalysisResult={pendingAnalysisResult} onDisplayPrioritizedActions={wrappedDisplayPrioritized} onRunAnalysis={wrappedRunAnalysis} + onCancelAnalysis={analysis.cancelAnalysis} canRunAnalysis={selectedContingency.length > 0 && !analysisLoading} onActionSelect={wrappedActionSelect} onActionFavorite={wrappedActionFavorite} diff --git a/frontend/src/api.test.ts b/frontend/src/api.test.ts index 702c50b5..067c2f4c 100644 --- a/frontend/src/api.test.ts +++ b/frontend/src/api.test.ts @@ -254,6 +254,7 @@ describe('api client', () => { expect(mockedAxios.post).toHaveBeenCalledWith( 'http://127.0.0.1:8000/api/run-analysis-step1', { disconnected_elements: ['LINE_X'] }, + { signal: undefined }, ); expect(result).toEqual(responseData); }); diff --git a/frontend/src/api.ts b/frontend/src/api.ts index a212355d..fe0f7c7d 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -315,8 +315,8 @@ export const api = { }); return response.data; }, - runAnalysisStep1: async (disconnectedElements: string[]): Promise<{ lines_overloaded: string[]; message: string; can_proceed: boolean }> => { - const response = await axios.post(`${API_BASE_URL}/api/run-analysis-step1`, { disconnected_elements: disconnectedElements }); + runAnalysisStep1: async (disconnectedElements: string[], signal?: AbortSignal): Promise<{ lines_overloaded: string[]; message: string; can_proceed: boolean }> => { + const response = await axios.post(`${API_BASE_URL}/api/run-analysis-step1`, { disconnected_elements: disconnectedElements }, { signal }); return response.data; }, runAnalysisStep2Stream: async (params: { @@ -324,7 +324,7 @@ export const api = { all_overloads: string[]; monitor_deselected: boolean; additional_lines_to_cut?: string[]; - }): Promise => { + }, signal?: AbortSignal): Promise => { const response = await fetch(`${API_BASE_URL}/api/run-analysis-step2`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -332,6 +332,7 @@ export const api = { ...params, additional_lines_to_cut: params.additional_lines_to_cut ?? [], }), + signal, }); if (!response.ok) { throw new Error(`Analysis Resolution failed: ${response.statusText}`); diff --git a/frontend/src/components/ActionFeed.test.tsx b/frontend/src/components/ActionFeed.test.tsx index 2d91e5ba..2875504e 100644 --- a/frontend/src/components/ActionFeed.test.tsx +++ b/frontend/src/components/ActionFeed.test.tsx @@ -104,6 +104,20 @@ describe('ActionFeed', () => { expect(screen.queryByTestId('make-first-guess-button')).not.toBeInTheDocument(); }); + it('shows a Cancel control while analyzing and invokes onCancelAnalysis (D5)', () => { + const onCancelAnalysis = vi.fn(); + render(); + const cancel = screen.getByTestId('cancel-analysis'); + expect(cancel).toBeInTheDocument(); + fireEvent.click(cancel); + expect(onCancelAnalysis).toHaveBeenCalledTimes(1); + }); + + it('omits the Cancel control when no onCancelAnalysis handler is provided', () => { + render(); + expect(screen.queryByTestId('cancel-analysis')).not.toBeInTheDocument(); + }); + it('hides the button while a pending analysis result is awaiting display', () => { const pending = { actions: {}, diff --git a/frontend/src/components/ActionFeed.tsx b/frontend/src/components/ActionFeed.tsx index 3918b94b..77cf7fe4 100644 --- a/frontend/src/components/ActionFeed.tsx +++ b/frontend/src/components/ActionFeed.tsx @@ -37,6 +37,9 @@ interface ActionFeedProps { scrollTarget?: { id: string; seq: number } | null; onDisplayPrioritizedActions: () => void; onRunAnalysis: () => void; + /** Abort an in-flight analysis run. When provided, a Cancel control + * appears alongside the "Analyzing…" indicator (D5). */ + onCancelAnalysis?: () => void; canRunAnalysis: boolean; onActionSelect: (actionId: string | null) => void; onActionFavorite: (actionId: string) => void; @@ -145,6 +148,7 @@ const ActionFeed: React.FC = ({ pendingAnalysisResult, onDisplayPrioritizedActions, onRunAnalysis, + onCancelAnalysis, canRunAnalysis, onActionSelect, onActionFavorite, @@ -1132,15 +1136,31 @@ const ActionFeed: React.FC = ({ {(analysisLoading || pendingAnalysisResult || (prioritizedEntries.length === 0 && overviewFilteredOutCount === 0)) && (
{analysisLoading ? ( - + <> + + {onCancelAnalysis && ( + + )} + ) : pendingAnalysisResult ? (
diff --git a/frontend/src/components/ActionFeed.test.tsx b/frontend/src/components/ActionFeed.test.tsx index 2875504e..c89db414 100644 --- a/frontend/src/components/ActionFeed.test.tsx +++ b/frontend/src/components/ActionFeed.test.tsx @@ -2975,11 +2975,15 @@ describe('ActionFeed', () => { // extra cuts where the analysis is launched. It must // disappear the moment analysis is in flight or results are // pending — same gate as the button itself. + // D4 consolidation: the picker wiring is one grouped `additionalLines` + // object on ActionFeed instead of four flat props. const pickerProps = { - branches: ['LINE_A', 'LINE_B', 'LINE_C'], - additionalLinesToCut: new Set(), - onToggleAdditionalLineToCut: vi.fn(), - n1Overloads: ['LINE_C'], + additionalLines: { + branches: ['LINE_A', 'LINE_B', 'LINE_C'], + additionalLinesToCut: new Set(), + onToggleAdditionalLineToCut: vi.fn(), + n1Overloads: ['LINE_C'], + }, }; it('renders the picker alongside Analyze & Suggest in the empty state', () => { @@ -3132,13 +3136,17 @@ describe('ActionFeed', () => { // model that produced the suggestions is reminded below the // Suggested Actions tab header alongside a danger-coloured Clear // button. + // D4 consolidation: the model dropdown wiring is one grouped + // `modelSelector` object on ActionFeed instead of flat props. const modelProps = { - recommenderModel: 'expert', - setRecommenderModel: vi.fn(), - availableModels: [ - { name: 'expert', label: 'Expert system', requires_overflow_graph: true, is_default: true, params: [] }, - { name: 'random_overflow', label: 'Random (post overflow analysis)', requires_overflow_graph: true, is_default: false, params: [] }, - ], + modelSelector: { + recommenderModel: 'expert', + setRecommenderModel: vi.fn(), + availableModels: [ + { name: 'expert', label: 'Expert system', requires_overflow_graph: true, is_default: true, params: [] }, + { name: 'random_overflow', label: 'Random (post overflow analysis)', requires_overflow_graph: true, is_default: false, params: [] }, + ], + }, }; const recAction = { @@ -3163,7 +3171,11 @@ describe('ActionFeed', () => { it('changing the model fires setRecommenderModel and logs recommender_model_changed', () => { const setRecommenderModel = vi.fn(); render( - , + , ); fireEvent.change(screen.getByLabelText('Model:'), { target: { value: 'random_overflow' } }); expect(setRecommenderModel).toHaveBeenCalledWith('random_overflow'); @@ -3184,7 +3196,7 @@ describe('ActionFeed', () => { , ); @@ -3198,7 +3210,7 @@ describe('ActionFeed', () => { , ); @@ -3210,13 +3222,15 @@ describe('ActionFeed', () => { , ); const total = screen.getByTestId('execution-time-total'); @@ -3238,10 +3252,12 @@ describe('ActionFeed', () => { , ); const total = screen.getByTestId('execution-time-total'); @@ -3258,7 +3274,7 @@ describe('ActionFeed', () => { , ); expect(screen.queryByTestId('execution-time-total')).not.toBeInTheDocument(); @@ -3270,7 +3286,7 @@ describe('ActionFeed', () => { , ); diff --git a/frontend/src/components/ActionFeed.tsx b/frontend/src/components/ActionFeed.tsx index 77cf7fe4..b3dff6d5 100644 --- a/frontend/src/components/ActionFeed.tsx +++ b/frontend/src/components/ActionFeed.tsx @@ -20,6 +20,56 @@ import ActionSearchDropdown from './ActionSearchDropdown'; import { colors } from '../styles/tokens'; import { apiErrorMessage } from '../utils/apiError'; +/** + * Per-stage analysis execution-time breakdown (seconds), grouped (D4) + * instead of six flat props. All are echoed by the backend in the step-2 + * ``result`` event and rendered together as the one-line breakdown under + * "Suggestions produced by …". ``overflowGraphTime`` is ``null`` when the + * active model does not consume the overflow graph (its column is hidden + * in that case). ``step1Time`` is the contingency simulation that runs + * before step 2; ``enrichmentTime`` is the Co-Study4Grid post-processing + * after assessment. ``wallClockTime`` is the frontend-measured round-trip + * from the click to the result event. Optional as a whole. + */ +export interface AnalysisTimingControls { + overflowGraphTime?: number | null; + actionPredictionTime?: number | null; + assessmentTime?: number | null; + step1Time?: number | null; + enrichmentTime?: number | null; + wallClockTime?: number | null; +} + +/** + * Recommendation-model selector wiring, grouped (D4). The dropdown above + * "Analyze & Suggest" (mirror of the same control in the Settings modal) + * lets the operator pick a different model and re-run without opening + * Settings; ``activeModelLabel`` is the display label of the model that + * produced the currently-shown suggestions (echoed by the backend in the + * step-2 ``result`` event as ``active_model``). Optional as a whole. + */ +export interface ModelSelectorControls { + recommenderModel?: string; + setRecommenderModel?: (v: string) => void; + availableModels?: ModelDescriptor[]; + activeModelLabel?: string | null; +} + +/** + * "Additional lines to cut" picker wiring, grouped (D4): the catalogue of + * disconnectable elements feeding the "additional lines to prevent flow + * increase" picker rendered above the Analyze & Suggest button, the + * operator-selected extras (ExpertAgent's ``additionalLinesToCut``), and + * the N-1 detected overloads excluded from the picker suggestions. + * Optional as a whole. + */ +export interface AdditionalLinesControls { + branches?: string[]; + additionalLinesToCut?: Set; + onToggleAdditionalLineToCut?: (line: string) => void; + n1Overloads?: string[]; +} + interface ActionFeedProps { actions: Record; actionScores?: Record>; @@ -84,50 +134,12 @@ interface ActionFeedProps { overviewFilters?: ActionOverviewFilters; /** Update the shared filter state (owned by App.tsx). */ onOverviewFiltersChange?: (next: ActionOverviewFilters) => void; - /** - * Catalogue of disconnectable elements feeding the - * "additional lines to prevent flow increase" picker rendered - * above the Analyze & Suggest button. - */ - branches?: string[]; - /** Operator-selected extras (ExpertAgent's `additionalLinesToCut`). */ - additionalLinesToCut?: Set; - onToggleAdditionalLineToCut?: (line: string) => void; - /** N-1 detected overloads — excluded from the picker suggestions. */ - n1Overloads?: string[]; - /** - * Recommendation model selector exposed above "Analyze & Suggest" - * (mirror of the same control in the Settings modal). Lets the - * operator pick a different model and re-run without opening - * Settings. - */ - recommenderModel?: string; - setRecommenderModel?: (v: string) => void; - availableModels?: ModelDescriptor[]; - /** - * Display label of the model that produced the currently-shown - * suggestions (echoed by the backend in the step-2 `result` event - * as ``active_model``). Rendered just below the "Suggested Actions" - * tab header alongside the Clear button. - */ - activeModelLabel?: string | null; - /** - * Per-stage execution times (seconds) for the current analysis, - * echoed by the backend in the step-2 ``result`` event. Rendered as - * a one-line breakdown right under "Suggestions produced by …". - * ``overflowGraphTime`` is ``null`` when the active model does not - * consume the overflow graph (its column is hidden in that case). - * ``step1Time`` is the contingency simulation that runs before - * step 2; ``enrichmentTime`` is the Co-Study4Grid post-processing - * after assessment. ``wallClockTime`` is the frontend-measured - * round-trip from the click to the result event. - */ - overflowGraphTime?: number | null; - actionPredictionTime?: number | null; - assessmentTime?: number | null; - step1Time?: number | null; - enrichmentTime?: number | null; - wallClockTime?: number | null; + /** "Additional lines to cut" picker wiring, grouped (D4). See AdditionalLinesControls. */ + additionalLines?: AdditionalLinesControls; + /** Recommendation-model selector wiring, grouped (D4). See ModelSelectorControls. */ + modelSelector?: ModelSelectorControls; + /** Per-stage analysis execution-time breakdown, grouped (D4). See AnalysisTimingControls. */ + timing?: AnalysisTimingControls; /** * Clear the un-touched recommender suggestions — wipes entries * still in ``suggestedByRecommenderIds`` that the operator has @@ -169,22 +181,34 @@ const ActionFeed: React.FC = ({ onActionDiagramPrimed, voltageLevelsLength, overviewFilters, - branches, - additionalLinesToCut, - onToggleAdditionalLineToCut, - n1Overloads, - recommenderModel, - setRecommenderModel, - availableModels, - activeModelLabel, - overflowGraphTime, - actionPredictionTime, - assessmentTime, - step1Time, - enrichmentTime, - wallClockTime, + additionalLines, + modelSelector, + timing, onClearSuggested, }) => { + // Unpack the grouped D4 controls so the body below keeps referring to + // the individual fields unchanged (defaults preserved from the former + // flat props). + const { + branches, + additionalLinesToCut, + onToggleAdditionalLineToCut, + n1Overloads, + } = additionalLines ?? {}; + const { + recommenderModel, + setRecommenderModel, + availableModels, + activeModelLabel, + } = modelSelector ?? {}; + const { + overflowGraphTime, + actionPredictionTime, + assessmentTime, + step1Time, + enrichmentTime, + wallClockTime, + } = timing ?? {}; const [searchOpen, setSearchOpen] = useState(false); const [searchQuery, setSearchQuery] = useState(''); const [combineModalOpen, setCombineModalOpen] = useState(false); diff --git a/frontend/src/components/VisualizationPanel.test.tsx b/frontend/src/components/VisualizationPanel.test.tsx index c63de712..e34f1a67 100644 --- a/frontend/src/components/VisualizationPanel.test.tsx +++ b/frontend/src/components/VisualizationPanel.test.tsx @@ -14,6 +14,39 @@ import VisualizationPanel from './VisualizationPanel'; import type { DiagramData, AnalysisResult, TabId, ActionOverviewFilters } from '../types'; import { DEFAULT_ACTION_OVERVIEW_FILTERS } from '../utils/actionTypes'; +// D4 consolidation: VisualizationPanel groups the detached-tabs, overflow, +// and action-overview wiring into three cohesive object props (`detach`, +// `overflow`, `actionOverview`) instead of ~30 flat props. Most of the +// behavioural tests below predate that grouping and set the individual +// fields flat (e.g. `detachedTabs`, `overflowLayoutMode`, `onActionSelect`). +// Rather than nest every one by hand, `createDefaultProps` partitions any +// flat cluster key it recognises back into its grouped object — so each +// test keeps asserting on what it exercises (Tie button, layout toggle, …) +// without prop-nesting boilerplate. A test may also pass a whole +// `detach` / `overflow` / `actionOverview` object directly (see the grouped +// sldEdit tests for the same style); the flat-derived group is merged on top. +const CLUSTER_KEYS: Record = { + detach: ['detachedTabs', 'onDetachTab', 'onReattachTab', 'onFocusDetachedTab', 'isTabTied', 'onToggleTabTie'], + overflow: ['overflowLayoutMode', 'overflowLayoutLoading', 'onOverflowLayoutChange', 'onOverflowPinPreview', 'onOverflowPinDoubleClick', 'overflowPins', 'overflowPinsEnabled', 'onOverflowPinsToggle'], + actionOverview: ['n1MetaIndex', 'onActionSelect', 'onActionFavorite', 'onActionReject', 'selectedActionIds', 'rejectedActionIds', 'onPinPreview', 'onOverviewPzChange', 'monitoringFactor', 'displayName', 'overviewFilters', 'onOverviewFiltersChange', 'sidebarCollapsed', 'hasActions', 'unsimulatedActionIds', 'unsimulatedActionInfo', 'onSimulateUnsimulatedAction'], +}; + +const groupClusterOverrides = (overrides: Record) => { + const groups: Record> = { detach: {}, overflow: {}, actionOverview: {} }; + const rest: Record = {}; + for (const [key, value] of Object.entries(overrides)) { + const groupName = Object.keys(CLUSTER_KEYS).find(g => CLUSTER_KEYS[g].includes(key)); + if (groupName) groups[groupName][key] = value; + else rest[key] = value; + } + return { + ...rest, + ...(Object.keys(groups.detach).length ? { detach: groups.detach } : {}), + ...(Object.keys(groups.overflow).length ? { overflow: groups.overflow } : {}), + ...(Object.keys(groups.actionOverview).length ? { actionOverview: groups.actionOverview } : {}), + }; +}; + const createDefaultProps = (overrides: Record = {}) => ({ activeTab: 'n' as TabId, configLoading: false, @@ -50,7 +83,7 @@ const createDefaultProps = (overrides: Record = {}) => ({ networkPath: '', layoutPath: '', onOpenSettings: vi.fn(), - ...overrides, + ...groupClusterOverrides(overrides), }); describe('VisualizationPanel', () => { diff --git a/frontend/src/components/VisualizationPanel.tsx b/frontend/src/components/VisualizationPanel.tsx index 57e3dcf3..f4f64442 100644 --- a/frontend/src/components/VisualizationPanel.tsx +++ b/frontend/src/components/VisualizationPanel.tsx @@ -67,6 +67,65 @@ export interface SldEditControls { onSldNavigateToVl?: (vlId: string) => void; } +/** + * Detached-tabs wiring, grouped (D4). Optional as a whole — call sites / + * tests that don't use the detach feature omit it. Built by App from + * `useDetachedTabs` / `useTiedTabsSync`. + */ +export interface DetachControls { + /** Map of tabs currently living in a secondary browser window. */ + detachedTabs?: DetachedTabsMap; + onDetachTab?: (tab: TabId) => void; + onReattachTab?: (tab: TabId) => void; + onFocusDetachedTab?: (tab: TabId) => void; + /** True iff the given tab is currently "tied" to the main window's activeTab. */ + isTabTied?: (tab: TabId) => boolean; + onToggleTabTie?: (tab: TabId) => void; +} + +/** + * Overflow Analysis tab wiring, grouped (D4): the hierarchical/geo layout + * toggle plus the action-pin overlay controls. Optional as a whole. + */ +export interface OverflowControls { + overflowLayoutMode?: 'hierarchical' | 'geo'; + overflowLayoutLoading?: boolean; + onOverflowLayoutChange?: (mode: 'hierarchical' | 'geo') => void; + /** Single click on an overflow-graph action pin — focus the feed card. */ + onOverflowPinPreview?: (actionId: string) => void; + /** Double-click on an action pin — route to the post-action SLD. */ + onOverflowPinDoubleClick?: (actionId: string, substation: string) => void; + /** Pre-computed pin descriptors posted to the overflow-graph iframe. */ + overflowPins?: ReadonlyArray; + overflowPinsEnabled?: boolean; + onOverflowPinsToggle?: (enabled: boolean) => void; +} + +/** + * Action-overview wiring, grouped (D4): the pin-interaction handlers, + * selection/rejection sets, filter state, and un-simulated pin metadata + * the Remedial Action overview view reads. Optional as a whole. + */ +export interface ActionOverviewControls { + n1MetaIndex?: MetadataIndex | null; + onActionSelect?: (actionId: string | null) => void; + onActionFavorite?: (actionId: string) => void; + onActionReject?: (actionId: string) => void; + selectedActionIds?: Set; + rejectedActionIds?: Set; + onPinPreview?: (actionId: string) => void; + onOverviewPzChange?: (pz: PZInstance) => void; + monitoringFactor?: number; + displayName?: (id: string) => string; + overviewFilters?: ActionOverviewFilters; + onOverviewFiltersChange?: (next: ActionOverviewFilters) => void; + sidebarCollapsed?: boolean; + hasActions?: boolean; + unsimulatedActionIds?: readonly string[]; + unsimulatedActionInfo?: Readonly>; + onSimulateUnsimulatedAction?: (actionId: string) => void; +} + interface VisualizationPanelProps { activeTab: TabId; configLoading: boolean; @@ -100,15 +159,8 @@ interface VisualizationPanelProps { * is currently detached. */ onViewModeChangeForTab?: (tab: TabId, mode: 'network' | 'delta') => void; - /** - * Overflow Analysis tab layout toggle (Hierarchical / Geo). Only - * rendered on the overflow tab when an overflow file is present. - * Cache-backed on the backend — switching between modes is - * instant after the first regeneration. - */ - overflowLayoutMode?: 'hierarchical' | 'geo'; - overflowLayoutLoading?: boolean; - onOverflowLayoutChange?: (mode: 'hierarchical' | 'geo') => void; + /** Overflow Analysis tab wiring (layout toggle + action pins), grouped (D4). See OverflowControls. */ + overflow?: OverflowControls; inspectQuery: string; onInspectQueryChange: (query: string) => void; /** @@ -132,91 +184,13 @@ interface VisualizationPanelProps { sldEdit?: SldEditControls; voltageLevels: string[]; onVlOpen: (vlName: string) => void; - /** - * Single click on an overflow-graph action pin. Should focus the - * feed card for that action (same as clicking an Action-Overview - * pin) — NOT switch tabs. Optional so call sites that don't - * surface the overflow viewer can omit it. - */ - onOverflowPinPreview?: (actionId: string) => void; - /** - * Double-click on an action pin in the overflow graph. Routes the - * user to the SLD overlay opened on the post-action ('action') - * sub-tab for that substation. Optional so call sites that don't - * surface the overflow viewer can omit it. - */ - onOverflowPinDoubleClick?: (actionId: string, substation: string) => void; networkPath: string; layoutPath: string; onOpenSettings: (tab?: 'recommender' | 'configurations' | 'paths') => void; - /** - * Map of tabs currently living in a secondary browser window. - * Optional so existing call sites / tests that don't care about the - * detach feature continue to work without wiring the props up. - */ - detachedTabs?: DetachedTabsMap; - /** Detach a tab into a new popup window. */ - onDetachTab?: (tab: TabId) => void; - /** Close the popup for a tab and fold it back inline. */ - onReattachTab?: (tab: TabId) => void; - /** Bring focus to the popup window hosting a detached tab. */ - onFocusDetachedTab?: (tab: TabId) => void; - /** True iff the given tab is currently "tied" to the main window's activeTab. */ - isTabTied?: (tab: TabId) => boolean; - /** Toggle the "tied" flag for the given tab. */ - onToggleTabTie?: (tab: TabId) => void; - /** - * Metadata index for the N-1 diagram. Used by the - * action-overview view (shown in the Remedial Action tab - * when no card is selected) to resolve each action to an - * (x, y) anchor on the network. - */ - n1MetaIndex?: MetadataIndex | null; - /** - * Invoked when the user clicks a pin on the action-overview - * view — should trigger the same action-select flow as - * clicking a card in the sidebar. Accepts `null` so the - * top-right ✕ button on the action tab can deselect. - */ - onActionSelect?: (actionId: string | null) => void; - /** Toggle an action's favourite (starred) status. */ - onActionFavorite?: (actionId: string) => void; - /** Reject an action from the suggestions feed. */ - onActionReject?: (actionId: string) => void; - /** Selected-action id set (for the overview popover styling). */ - selectedActionIds?: Set; - /** Rejected-action id set (for the overview popover styling). */ - rejectedActionIds?: Set; - /** Called when a pin is single-clicked on the overview (scroll sidebar to card). */ - onPinPreview?: (actionId: string) => void; - /** Called when the overview's usePanZoom instance changes (for tied-tab sync). */ - onOverviewPzChange?: (pz: PZInstance) => void; - /** - * Monitoring factor used to derive each action's severity - * colour, kept in sync with the card palette. - */ - monitoringFactor?: number; - /** Resolve an element/VL ID to its human-readable display name. */ - displayName?: (id: string) => string; - /** Shared category + threshold + un-simulated filter state. */ - overviewFilters?: ActionOverviewFilters; - /** Setter for the shared overview filter state (owned by App.tsx). */ - onOverviewFiltersChange?: (next: ActionOverviewFilters) => void; - /** When true, the sidebar is collapsed and its ActionFilterRings - * row is unreachable. Surface a copy of the strip on the left - * side of this panel's tab row so the filter remains accessible - * even with the sidebar folded away. */ - sidebarCollapsed?: boolean; - /** Whether the action feed currently holds any card to filter — - * the inline filter strip stays hidden until there is something - * to act on, mirroring the sidebar's gating. */ - hasActions?: boolean; - /** Ids of scored-but-not-simulated actions to render as dimmed pins. */ - unsimulatedActionIds?: readonly string[]; - /** Per-id score metadata used to enrich the un-simulated pin tooltip. */ - unsimulatedActionInfo?: Readonly>; - /** Kick off a manual simulation when an un-simulated pin is double-clicked. */ - onSimulateUnsimulatedAction?: (actionId: string) => void; + /** Detached-tabs wiring, grouped (D4). See DetachControls. */ + detach?: DetachControls; + /** Action-overview wiring (pins, selection, filters), grouped (D4). See ActionOverviewControls. */ + actionOverview?: ActionOverviewControls; /** * When false, voltage-level labels rendered by pypowsybl inside the * NAD's `` are hidden via CSS. @@ -225,18 +199,6 @@ interface VisualizationPanelProps { */ showVoltageLevelNames?: boolean; onToggleVoltageLevelNames?: (show: boolean) => void; - /** - * Pre-computed pin descriptors posted to the overflow-graph - * iframe when `overflowPinsEnabled` is true. Built upstream by - * `buildOverflowPinPayload` from `actions`, `monitoringFactor`, - * and the VL→substation map. Re-posted whenever the array - * reference changes. - */ - overflowPins?: ReadonlyArray; - /** Toggle state — controlled by App.tsx so it survives tab swaps. */ - overflowPinsEnabled?: boolean; - /** Setter for `overflowPinsEnabled` — wired to interactionLogger. */ - onOverflowPinsToggle?: (enabled: boolean) => void; } @@ -272,47 +234,19 @@ const VisualizationPanel: React.FC = ({ onOverlayClose, onOverlaySldTabChange, sldEdit, + detach, + overflow, + actionOverview, voltageLevels, onVlOpen, - onOverflowPinPreview, - onOverflowPinDoubleClick, networkPath, layoutPath, onOpenSettings, - detachedTabs = {}, - onDetachTab, - onReattachTab, - onFocusDetachedTab, onInspectQueryChangeFor, viewModeForTab, onViewModeChangeForTab, - overflowLayoutMode, - overflowLayoutLoading, - onOverflowLayoutChange, - isTabTied, - onToggleTabTie, - n1MetaIndex, - onActionSelect, - onActionFavorite, - onActionReject, - selectedActionIds, - rejectedActionIds, - onPinPreview, - onOverviewPzChange, - monitoringFactor, - displayName, - overviewFilters, - onOverviewFiltersChange, - sidebarCollapsed, - hasActions, - unsimulatedActionIds, - unsimulatedActionInfo, - onSimulateUnsimulatedAction, showVoltageLevelNames = true, onToggleVoltageLevelNames, - overflowPins, - overflowPinsEnabled = false, - onOverflowPinsToggle, }) => { // Unpack the grouped SLD-edit controls (D4) so the body below keeps // referring to the individual fields unchanged. @@ -340,6 +274,46 @@ const VisualizationPanel: React.FC = ({ onSldSwitchRemoveMany, onSldNavigateToVl, } = sldEdit ?? {}; + // Detached-tabs / overflow / action-overview controls, grouped (D4) — + // unpacked here so the body below keeps referring to the individual + // fields unchanged (defaults preserved from the former props). + const { + detachedTabs = {}, + onDetachTab, + onReattachTab, + onFocusDetachedTab, + isTabTied, + onToggleTabTie, + } = detach ?? {}; + const { + overflowLayoutMode, + overflowLayoutLoading, + onOverflowLayoutChange, + onOverflowPinPreview, + onOverflowPinDoubleClick, + overflowPins, + overflowPinsEnabled = false, + onOverflowPinsToggle, + } = overflow ?? {}; + const { + n1MetaIndex, + onActionSelect, + onActionFavorite, + onActionReject, + selectedActionIds, + rejectedActionIds, + onPinPreview, + onOverviewPzChange, + monitoringFactor, + displayName, + overviewFilters, + onOverviewFiltersChange, + sidebarCollapsed, + hasActions, + unsimulatedActionIds, + unsimulatedActionInfo, + onSimulateUnsimulatedAction, + } = actionOverview ?? {}; // No-op fallbacks so conditional branches don't need to guard. const detachTabCb = onDetachTab ?? (() => {}); From df66d8164508f0968c52286318259f2e8c456cd2 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 11:58:03 +0000 Subject: [PATCH 13/24] test(D4): explicit grouped-prop contract tests for the consolidated props MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add direct-object contract tests locking the D4 props consolidation on both hub-fed components. Each test passes the grouped object DIRECTLY (bypassing the flat-override partition helper) and asserts field-by-field forwarding plus the "optional as a whole" omission case: - VisualizationPanel: detach (Tie button + toggle callback), overflow (Hierarchical/Geo layout toggle + callback), actionOverview (collapsed inline filter strip) — each with its omitted-group negative case. - ActionFeed: additionalLines (picker), modelSelector (Model dropdown + change callback), timing (execution-time total) — each with its omitted-group negative case. Full Vitest suite green (VisualizationPanel 65 -> 71, ActionFeed 121 -> 127); review tracker D4 note updated to cite the explicit contract tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- docs/architecture/2026-07-full-repo-review.md | 8 +- frontend/src/components/ActionFeed.test.tsx | 91 +++++++++++++++++++ .../components/VisualizationPanel.test.tsx | 88 ++++++++++++++++++ 3 files changed, 185 insertions(+), 2 deletions(-) diff --git a/docs/architecture/2026-07-full-repo-review.md b/docs/architecture/2026-07-full-repo-review.md index d57678b5..fc9412e1 100644 --- a/docs/architecture/2026-07-full-repo-review.md +++ b/docs/architecture/2026-07-full-repo-review.md @@ -679,9 +679,13 @@ already uses). Then *lower* `APP_TSX_MAX` to lock in the win. > (4 picker props), `modelSelector?: ModelSelectorControls` (4 model-dropdown > props), `timing?: AnalysisTimingControls` (6 execution-time props). > -> All behaviour-preserving (full Vitest suite green — 1,771 specs); guarded by +> All behaviour-preserving (full Vitest suite green); guarded by > `useManualSimulation` / `useOverflowLayout` / `useActionDiagramCache` -> hook tests + the `VisualizationPanel` / `ActionFeed` grouped-prop tests. +> hook tests + explicit `VisualizationPanel` / `ActionFeed` grouped-prop +> contract tests that pass each group object directly and assert both +> field-by-field forwarding **and** the "optional as a whole" omission +> case (detach / overflow / actionOverview + additionalLines / +> modelSelector / timing). > **Remaining tail**: the deeply-coupled useDiagrams *core* > (`handleActionSelect`, `zoomToElement`, the DOM-mutating voltage-filter > effects — moving effect order that tsc can't verify) is deferred and diff --git a/frontend/src/components/ActionFeed.test.tsx b/frontend/src/components/ActionFeed.test.tsx index c89db414..b15aba8e 100644 --- a/frontend/src/components/ActionFeed.test.tsx +++ b/frontend/src/components/ActionFeed.test.tsx @@ -3316,4 +3316,95 @@ describe('ActionFeed', () => { expect(screen.getByRole('button', { name: /Analyze & Suggest/ })).toBeInTheDocument(); }); }); + + // D4 consolidation: prove the three grouped ActionFeed props + // (additionalLines / modelSelector / timing) are unpacked and forwarded + // when passed as a whole object, and that each is safely OPTIONAL. These + // pass the grouped object DIRECTLY so they lock the object-prop contract. + describe('grouped additionalLines / modelSelector / timing props (D4 consolidation)', () => { + const recAction = { + description_unitaire: 'Disco LINE_X', + action_topology: emptyTopo, + is_manual: false, + max_rho: 0.8, + } as unknown as ActionDetail; + + it('threads a grouped additionalLines object through to the picker', () => { + render( + (), + onToggleAdditionalLineToCut: vi.fn(), + n1Overloads: ['LINE_B'], + }} + canRunAnalysis + />, + ); + expect(screen.getByTestId('additional-lines-picker')).toBeInTheDocument(); + }); + + it('omits the picker when the additionalLines group is absent', () => { + render(); + expect(screen.queryByTestId('additional-lines-picker')).not.toBeInTheDocument(); + }); + + it('threads a grouped modelSelector object through to the Model dropdown', () => { + const setRecommenderModel = vi.fn(); + render( + , + ); + const select = screen.getByLabelText('Model:') as HTMLSelectElement; + expect(select.value).toBe('expert'); + fireEvent.change(select, { target: { value: 'random_overflow' } }); + expect(setRecommenderModel).toHaveBeenCalledWith('random_overflow'); + }); + + it('omits the Model dropdown when the modelSelector group is absent', () => { + render(); + expect(screen.queryByLabelText('Model:')).not.toBeInTheDocument(); + }); + + it('threads a grouped timing object through to the execution-time total', () => { + render( + , + ); + expect(screen.getByTestId('execution-time-total')).toHaveTextContent(/5\.50s/); + }); + + it('omits the execution-time total when the timing group is absent', () => { + render( + , + ); + expect(screen.queryByTestId('execution-time-total')).not.toBeInTheDocument(); + }); + }); }); diff --git a/frontend/src/components/VisualizationPanel.test.tsx b/frontend/src/components/VisualizationPanel.test.tsx index e34f1a67..c6ae707f 100644 --- a/frontend/src/components/VisualizationPanel.test.tsx +++ b/frontend/src/components/VisualizationPanel.test.tsx @@ -688,6 +688,94 @@ describe('VisualizationPanel', () => { }); }); + // D4 consolidation: prove the three remaining grouped props (detach / + // overflow / actionOverview) are unpacked and forwarded when passed as a + // whole object — and that each is safely OPTIONAL (omitting it renders + // the panel without that affordance, the "optional as a whole" contract). + // These pass the grouped object DIRECTLY (bypassing the flat-override + // partition helper) so they lock the object-prop wire contract itself. + describe('grouped detach / overflow / actionOverview props (D4 consolidation)', () => { + const nDiagram: DiagramData = { svg: 'n', metadata: null }; + + it('threads a grouped detach object through to the Tie button', () => { + const mountNode = document.createElement('div'); + document.body.appendChild(mountNode); + const onToggleTabTie = vi.fn(); + render( false, + onToggleTabTie, + }, + })} />); + const tieButton = mountNode.querySelector('button[title^="Tie"]') as HTMLElement | null; + expect(tieButton).not.toBeNull(); + fireEvent.click(tieButton!); + expect(onToggleTabTie).toHaveBeenCalledWith('n'); + document.body.removeChild(mountNode); + }); + + it('renders no Tie affordance when the detach group is omitted', () => { + render(); + expect(document.body.querySelectorAll('button[title^="Tie"]')).toHaveLength(0); + }); + + it('threads a grouped overflow object through to the layout toggle', () => { + const onOverflowLayoutChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Geo' })); + expect(onOverflowLayoutChange).toHaveBeenCalledWith('geo'); + }); + + it('omits the layout toggle when the overflow group is omitted', () => { + render(); + expect(screen.queryByRole('button', { name: 'Geo' })).not.toBeInTheDocument(); + }); + + it('threads a grouped actionOverview object through to the collapsed filter strip', () => { + render(); + expect(screen.getByTestId('viz-panel-overview-filters')).toBeInTheDocument(); + }); + + it('renders no inline filter strip when the actionOverview group is omitted', () => { + render(); + expect(screen.queryByTestId('viz-panel-overview-filters')).not.toBeInTheDocument(); + }); + }); + // ===== Regression tests for auto-zoom double-injection fix ===== // The N / N-1 / action diagram containers (MemoizedSvgContainer) are kept // ALWAYS mounted with an empty-string placeholder. This prevents React From b33e8b09888c079f478ed52729904ed1712bed2a Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 12:11:37 +0000 Subject: [PATCH 14/24] =?UTF-8?q?perf(D6):=20SVG=20element-adoption=20pipe?= =?UTF-8?q?line=20=E2=80=94=20parse=20once,=20adopt=20directly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finish the element-adoption pipeline the review flagged: a full-NAD load paid an avoidable serialize + re-parse round-trip (boostSvgForLargeGrid serialized the mutated DOM back to a string; MemoizedSvgContainer then re-parsed it via innerHTML), and the component's "zero extra parse" comment was false on every string path. - svgBoost.ts: new boostSvgToElement(rawSvg, viewBox, vlCount) parses the pypowsybl SVG ONCE and mutates the live SVGSVGElement in place (no XMLSerializer). processSvg returns that element. The boost mutation body is unchanged — only the parse (moved to the top) and the return (element, not serialized string) differ. - MemoizedSvgContainer already had the replaceChildren(element) adoption path — proven in production by the svgPatch fast-path, which sets n1Diagram.svg to a cloned SVGSVGElement. The full-fetch paths now ride the same rail. Branch on typeof (not instanceof) so any parsed element adopts. - Safety: boostSvgToElement returns null on a parse error OR a non-SVG-namespaced root, so MemoizedSvgContainer falls back to innerHTML (the HTML parser namespaces it). Real pypowsybl output always declares xmlns. SLD overlays keep their own string->innerHTML path untouched. - boostSvgForLargeGrid(string)->string retained as a thin wrapper for the serialized-SVG unit tests; returns the input unchanged on skip so no re-serialization side effects leak. Tests: element-adoption cases in svgBoost.test.ts (element return, parse- error string fallback, namespace guard) + a MemoizedSvgContainer identity test (the mounted is the SAME instance processSvg returned — a re-parse could never produce that). Full Vitest suite green (1,789). Docs: review tracker D6 marked core-done (bitmap auto-enable deferred, validation-gated) + perf weakness row resolved; frontend CLAUDE.md SVG handling section documents the new lever. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- docs/architecture/2026-07-full-repo-review.md | 25 +++- frontend/CLAUDE.md | 16 ++- .../components/MemoizedSvgContainer.test.tsx | 19 ++++ .../src/components/MemoizedSvgContainer.tsx | 20 ++-- frontend/src/utils/svg/svgBoost.test.ts | 49 +++++++- frontend/src/utils/svg/svgBoost.ts | 107 ++++++++++++++---- frontend/src/utils/svgUtils.test.ts | 7 +- frontend/src/utils/svgUtils.ts | 2 +- 8 files changed, 206 insertions(+), 39 deletions(-) diff --git a/docs/architecture/2026-07-full-repo-review.md b/docs/architecture/2026-07-full-repo-review.md index fc9412e1..0c236dd4 100644 --- a/docs/architecture/2026-07-full-repo-review.md +++ b/docs/architecture/2026-07-full-repo-review.md @@ -412,7 +412,7 @@ render hygiene at the App.tsx level.* | Sev | Finding | Verdict | |---|---|---| -| high | Full-NAD loads pay an avoidable serialize + re-parse round-trip (`svgBoost.ts:621` serializes; every string path re-parses via innerHTML); `MemoizedSvgContainer`'s "zero extra parse" comment is false on all string paths | 🟡 — avoidable slice is several hundred ms per load, not the full 1.4 s | +| high | Full-NAD loads pay an avoidable serialize + re-parse round-trip (`svgBoost.ts:621` serializes; every string path re-parses via innerHTML); `MemoizedSvgContainer`'s "zero extra parse" comment is false on all string paths | 🟡 — avoidable slice is several hundred ms per load, not the full 1.4 s · ✅ **FIXED (D6, 2026-07-09)**: `processSvg` returns the boosted `SVGSVGElement`; `MemoizedSvgContainer` adopts it via `replaceChildren` — no serialize + `innerHTML` re-parse. Comment now accurate. | | med | Whole-object dependency churn re-runs the entire highlight pipeline after every pan/zoom settle | ✅ | | med | Default pan/zoom re-rasters the full vector layer per frame (~20 fps on the 5,247-VL grid) while the benchmarked 120 fps bitmap mode ships opt-in | 🟡 — opt-in is the *documented* shipped design pending real-hardware validation; a VL-count auto-enable is a reasonable follow-up, severity low-med | | med | Memory retention: 3 always-mounted 200k-node DOMs + unbounded per-action processed-SVG string cache + overview clone | 🟡 — bounded in practice (~100–300 MB worst case within one contingency; cache clears on contingency change); LRU cap still warranted | @@ -710,11 +710,32 @@ Fixes T4 at the root; QW4 below is the 2-hour stopgap. > surfacing as a cancellation notice rather than an error. **QW14** > (highlight-pipeline pan/zoom churn) is independent and still open. -**D6. Finish the SVG element-adoption pipeline** *(2–4 days)* +**D6. Finish the SVG element-adoption pipeline** *(2–4 days)* — 🟢 **CORE DONE (2026-07-09)** Make `processSvg` return the already-parsed `SVGSVGElement` (adoptNode) instead of re-serializing; route all ingestion paths through the element path that already exists for patches; then a VL-count heuristic to auto-select bitmap pan/zoom mode (or a discoverability nudge) once validated on operator hardware. +> **Shipped (element-adoption core)**: the boost is now element-based — +> `boostSvgToElement(rawSvg, viewBox, vlCount)` parses the pypowsybl SVG once +> and mutates the live `SVGSVGElement` in place (no `XMLSerializer` round-trip), +> and `processSvg` returns that element. `MemoizedSvgContainer` already had the +> `replaceChildren(element)` adoption path (proven by the svgPatch fast-path, +> which sets `n1Diagram.svg` to a cloned `SVGSVGElement`); the full-NAD load +> paths now ride the same rail, so a full load no longer pays the +> serialize + `innerHTML` re-parse the review flagged (`svgBoost.ts:621`). +> Safety: `boostSvgToElement` returns `null` on a parse error **or** a +> non-SVG-namespaced root, so `MemoizedSvgContainer` falls back to `innerHTML` +> (the HTML parser namespaces it) — the SLD-overlay string path is untouched. +> The old `boostSvgForLargeGrid(string) → string` stays as a thin wrapper for +> the serialized-SVG unit tests, returning the input unchanged on skip so no +> re-serialization leaks. Guarded by the element-adoption tests in +> `svgBoost.test.ts` + a `MemoizedSvgContainer` identity test (the mounted +> `` is the SAME instance `processSvg` returned — impossible via a +> re-parse). Full Vitest suite green. +> **Deferred (validation-gated)**: the VL-count auto-enable of the bitmap +> pan/zoom mode is intentionally left open — the review itself gates it on +> real operator-hardware validation of the bitmap engine (still OFF by +> default), so shipping an auto-enable blind would be premature. **D7. Deployment trust & reproducibility** *(3–4 days)* — 🟡 **MOSTLY DONE (2026-07-08)** Lockdown profile (env flag set in the Dockerfile) disabling/confining the diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 8476d473..4efd9b78 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -353,12 +353,26 @@ mirrored in `scripts/check_standalone_parity.py`'s `SPEC_DETAILS`. ## SVG handling -The frontend deals with multi-MB pypowsybl SVG payloads. Four +The frontend deals with multi-MB pypowsybl SVG payloads. Five performance levers are applied today: - **`api.getNetworkDiagram` uses `format=text`** (`api.ts:69-92`): fetches a JSON-header + raw-SVG-body response so the browser doesn't `JSON.parse` a 25 MB string. Saves ~500 ms on large grids. +- **Element-adoption ingestion** (`utils/svg/svgBoost.ts` + `boostSvgToElement` + `processSvg`, D6): `processSvg` parses the raw + pypowsybl SVG **once** into a live `SVGSVGElement`, applies the + large-grid boost in place (no `XMLSerializer`), and returns that + element. `MemoizedSvgContainer` adopts it via + `replaceChildren(element)` instead of re-parsing a serialized string + through `innerHTML` — removing the serialize + re-parse round-trip + full-NAD loads used to pay. This is the same element rail the + svgPatch fast-path already rides (`applyPatchToClone` returns an + `SVGSVGElement`). `processSvg` falls back to the raw **string** (and + `MemoizedSvgContainer` to `innerHTML`) when the SVG fails to parse or + its root is not SVG-namespaced; SLD overlays (`SldOverlay`) keep + their own string→`innerHTML` path. The `boostSvgForLargeGrid(string) + → string` wrapper is retained for the serialized-SVG unit tests. - **`getIdMap(container)`** (`utils/svg/idMap.ts`): cached `WeakMap>` so highlight passes don't re-scan `[id]` selectors. Invalidate via `invalidateIdMapCache` diff --git a/frontend/src/components/MemoizedSvgContainer.test.tsx b/frontend/src/components/MemoizedSvgContainer.test.tsx index 79c868cb..4054c434 100644 --- a/frontend/src/components/MemoizedSvgContainer.test.tsx +++ b/frontend/src/components/MemoizedSvgContainer.test.tsx @@ -12,6 +12,7 @@ import { createRef } from 'react'; import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import MemoizedSvgContainer from './MemoizedSvgContainer'; +import { processSvg } from '../utils/svgUtils'; describe('MemoizedSvgContainer', () => { it('renders a container div with correct id and class', () => { @@ -55,6 +56,24 @@ describe('MemoizedSvgContainer', () => { expect(containerRef.current?.querySelector('circle')).toBeTruthy(); }); + // ===== D6: the SVG element-adoption pipeline ===== + // processSvg hands us an already-parsed SVGSVGElement; MemoizedSvgContainer + // must adopt that exact element via replaceChildren (no serialize + + // innerHTML re-parse). Proven by identity: the mounted is the SAME + // instance processSvg returned, which innerHTML could never produce. + it('adopts the exact element processSvg returned (no re-parse round-trip)', () => { + const containerRef = createRef(); + const raw = ''; + const { svg } = processSvg(raw, 100); + expect(typeof svg).not.toBe('string'); // element rail engaged + render( + + ); + // Identity check: same element instance moved into the container. + expect(containerRef.current?.firstElementChild).toBe(svg); + expect(containerRef.current?.querySelector('#marker')).toBeTruthy(); + }); + it('uses correct id for different tabId values', () => { const containerRef = createRef(); const { container } = render( diff --git a/frontend/src/components/MemoizedSvgContainer.tsx b/frontend/src/components/MemoizedSvgContainer.tsx index 3f675a94..a536382a 100644 --- a/frontend/src/components/MemoizedSvgContainer.tsx +++ b/frontend/src/components/MemoizedSvgContainer.tsx @@ -9,9 +9,11 @@ import React, { type RefObject } from 'react'; import type { TabId } from '../types'; // Prevents React from diffing massive SVG DOM trees on every parent render. -// Uses replaceChildren(svgElement) instead of innerHTML to avoid the double-parse: -// OLD: string → XMLSerializer → string → browser parse (twice) -// NEW: DOMParser (in processSvg) → SVGSVGElement → replaceChildren (zero extra parse) +// Uses replaceChildren(svgElement) instead of innerHTML to avoid the re-parse: +// processSvg (D6) / applyPatchToClone hand us an already-parsed SVGSVGElement, +// which we adopt directly — no serialize + innerHTML re-parse round-trip. +// A plain string (e.g. SLD overlays not routed through processSvg, or a +// processSvg parse-failure fallback) still takes the innerHTML path. interface SvgContainerProps { svg: SVGSVGElement | string; containerRef: RefObject; @@ -27,12 +29,14 @@ const MemoizedSvgContainer = React.memo(({ svg, containerRef, display, tabId, hi if (!container || !svg) return; const start = performance.now(); - if (svg instanceof SVGSVGElement) { - // DOM-reuse path: move the already-parsed element directly — no second parse - container.replaceChildren(svg); - } else { - // Fallback for plain string SVGs (e.g. SLD overlays not going through processSvg) + if (typeof svg === 'string') { + // Fallback for plain string SVGs (SLD overlays not going through + // processSvg, or a processSvg parse-failure fallback). container.innerHTML = svg; + } else { + // Element-adoption path: adopt the already-parsed element directly + // (auto-adopted across documents by replaceChildren) — no re-parse. + container.replaceChildren(svg); } console.log(`[SVG] DOM injection for ${tabId} took ${(performance.now() - start).toFixed(2)}ms`); }, [svg, containerRef, tabId]); diff --git a/frontend/src/utils/svg/svgBoost.test.ts b/frontend/src/utils/svg/svgBoost.test.ts index b20f37d7..6cc6a298 100644 --- a/frontend/src/utils/svg/svgBoost.test.ts +++ b/frontend/src/utils/svg/svgBoost.test.ts @@ -5,7 +5,7 @@ // SPDX-License-Identifier: MPL-2.0 import { describe, expect, it } from 'vitest'; -import { boostSvgForLargeGrid, processSvg } from './svgBoost'; +import { boostSvgForLargeGrid, boostSvgToElement, processSvg } from './svgBoost'; describe('boostSvgForLargeGrid', () => { const stableSvg = ''; @@ -204,7 +204,11 @@ describe('processSvg', () => { const svg = ''; const { viewBox, svg: out } = processSvg(svg, 100); expect(viewBox).toBeNull(); - expect(out).toBe(svg); + // D6: a well-formed SVG comes back as a live element (adopted + // directly), not a re-serialized string. The element preserves the + // root even without a viewBox. + expect(typeof out).not.toBe('string'); + expect((out as SVGSVGElement).nodeName.toLowerCase()).toBe('svg'); }); it('accepts a comma-separated viewBox', () => { @@ -213,3 +217,44 @@ describe('processSvg', () => { expect(viewBox).toEqual({ x: 0, y: 0, w: 100, h: 100 }); }); }); + +// D6 — the SVG element-adoption pipeline. processSvg / boostSvgToElement +// hand ingestion a live SVGSVGElement so MemoizedSvgContainer adopts it via +// replaceChildren instead of re-parsing a serialized string. +describe('SVG element-adoption pipeline (D6)', () => { + it('processSvg returns a live SVGSVGElement for a well-formed SVG', () => { + const svg = ''; + const { svg: out } = processSvg(svg, 100); + expect(typeof out).not.toBe('string'); + const el = out as SVGSVGElement; + expect(el.nodeName.toLowerCase()).toBe('svg'); + // Content is preserved on the parsed element. + expect(el.querySelector('#x')).not.toBeNull(); + }); + + it('processSvg falls back to the raw string on a parse error', () => { + // A non-SVG / malformed root can\'t be adopted — keep the string so + // MemoizedSvgContainer\'s innerHTML path still renders it. + const notSvg = '
not an svg
'; + const { svg: out } = processSvg(notSvg, 100); + expect(out).toBe(notSvg); + }); + + it('boostSvgToElement returns the parsed element unmutated for small grids', () => { + const svg = ''; + const el = boostSvgToElement(svg, { x: 0, y: 0, w: 100, h: 100 }, 100); + expect(el).not.toBeNull(); + expect(el!.nodeName.toLowerCase()).toBe('svg'); + expect(el!.querySelector('#keep')).not.toBeNull(); + }); + + it('boostSvgToElement returns null on a parse error', () => { + expect(boostSvgToElement('
nope
', null, 100)).toBeNull(); + }); + + it('boostSvgForLargeGrid string wrapper still returns the input unchanged when boost is skipped', () => { + const svg = ''; + // vlCount below the 500 floor → pass-through, byte-identical. + expect(boostSvgForLargeGrid(svg, { x: 0, y: 0, w: 100, h: 100 }, 100)).toBe(svg); + }); +}); diff --git a/frontend/src/utils/svg/svgBoost.ts b/frontend/src/utils/svg/svgBoost.ts index decc0260..e1c2e05f 100644 --- a/frontend/src/utils/svg/svgBoost.ts +++ b/frontend/src/utils/svg/svgBoost.ts @@ -11,18 +11,59 @@ import { declutterEdgeInfoLabels, parseRotateDeg, type EdgeInfoLabel } from './e * Scale SVG elements for large grids so text, nodes, and flow values * are readable when zoomed in and naturally shrink at full view. */ -export const boostSvgForLargeGrid = (svgString: string, viewBox: ViewBox | null, vlCount: number): string => { - if (!viewBox) return svgString; +// Grids below this rendered-viewBox / VL-density regime don't need the boost. +const REFERENCE_SIZE = 1250; +const BOOST_THRESHOLD = 3; - // Skip boost entirely for grids with < 500 voltage levels - if (!vlCount || vlCount < 500) return svgString; +/** + * Parse a raw pypowsybl SVG string into a live `SVGSVGElement`, applying the + * large-grid boost (font / node-radius scaling + flow-value de-clutter) in + * place, and return that element (D6 — the SVG element-adoption pipeline). + * + * Returning the parsed element — rather than a re-serialized string — lets the + * ingestion path (`MemoizedSvgContainer.replaceChildren`) adopt it directly, + * skipping the serialize + `innerHTML` re-parse round-trip the string path + * paid on every full-NAD load. This is the same element rail the svgPatch + * fast-path already rides (`applyPatchToClone` → `SVGSVGElement`). + * + * Returns `null` on a parse error so the caller can fall back to the raw + * string (`innerHTML`). For small / narrow-viewBox grids the boost is skipped + * and the parsed element is returned UNMUTATED (still an element → still no + * re-parse downstream). + */ +export const boostSvgToElement = (svgString: string, viewBox: ViewBox | null, vlCount: number): SVGSVGElement | null => { + let svgEl: SVGSVGElement; + try { + const parser = new DOMParser(); + const doc = parser.parseFromString(svgString, 'image/svg+xml'); + if (doc.getElementsByTagName('parsererror').length > 0) return null; + const root = doc.documentElement; + // Only adopt a root that is a genuine SVG-namespaced . A missing + // xmlns (or a non-svg root) would render blank if adopted directly — + // return null so the caller falls back to innerHTML, which the HTML + // parser namespaces for us. Real pypowsybl NAD/SLD output always + // declares xmlns="http://www.w3.org/2000/svg". + if ( + !root || + root.nodeName.toLowerCase() !== 'svg' || + root.namespaceURI !== 'http://www.w3.org/2000/svg' + ) { + return null; + } + svgEl = root as unknown as SVGSVGElement; + } catch (err) { + console.error('[SVG] Failed to parse SVG:', err); + return null; + } + + // Skip the boost for small grids / narrow viewBoxes — return the parsed + // element as-is (unmutated, but still an element so ingestion adopts it). + if (!viewBox || !vlCount || vlCount < 500) return svgEl; const start = Date.now(); const diagramSize = Math.max(viewBox.w, viewBox.h); - const REFERENCE_SIZE = 1250; - const BOOST_THRESHOLD = 3; const ratio = diagramSize / REFERENCE_SIZE; - if (ratio <= BOOST_THRESHOLD) return svgString; + if (ratio <= BOOST_THRESHOLD) return svgEl; const boost = Math.sqrt(ratio / BOOST_THRESHOLD); const boostStr = boost.toFixed(2); @@ -138,9 +179,8 @@ export const boostSvgForLargeGrid = (svgString: string, viewBox: ViewBox | null, const nodeBoostStr = nodeBoost.toFixed(2); try { - const parser = new DOMParser(); - const doc = parser.parseFromString(svgString, 'image/svg+xml'); - const svgEl = doc.documentElement; + // svgEl was already parsed at the top of this function — the boost + // mutates it in place (no second parse, no serialize). // === 1. Scale CSS values in
= ({ sessionRestoring, onRestoreSession, }) => { + // Escape-to-close + focus trap/restore (QW20). Called before the early + // return so hook order stays stable across open/closed renders. + const { containerRef, dialogProps } = useModalKeyboard({ + isOpen: showReloadModal, + onClose: () => setShowReloadModal(false), + }); + if (!showReloadModal) return null; return ( @@ -34,7 +42,10 @@ const ReloadSessionModal: React.FC = ({ backgroundColor: 'rgba(0,0,0,0.5)', zIndex: 3500, display: 'flex', justifyContent: 'center', alignItems: 'center' }}> -
= ({ settings, onApply }) => { handleCloseSettings, } = settings; + // Escape-to-close + focus trap/restore (QW20). Called before the early + // return so hook order stays stable across open/closed renders. + const { containerRef, dialogProps } = useModalKeyboard({ + isOpen: isSettingsOpen, + onClose: handleCloseSettings, + }); + if (!isSettingsOpen) return null; // Resolve the active model descriptor (or null when the registry is @@ -108,7 +116,8 @@ const SettingsModal: React.FC = ({ settings, onApply }) => { display: 'flex', justifyContent: 'center', alignItems: 'center' }}>
{ expect(popupAction.closed).toBe(true); }); }); + + describe('theme propagation into popups (QW19)', () => { + afterEach(() => { + document.documentElement.removeAttribute('data-theme'); + }); + + it('mirrors the opener theme onto the popup document at detach time', () => { + document.documentElement.setAttribute('data-theme', 'dark'); + const { result } = renderHook(() => useDetachedTabs()); + act(() => { result.current.detach('contingency'); }); + const doc = fakePopups[0].document; + expect(doc.documentElement.getAttribute('data-theme')).toBe('dark'); + expect(doc.documentElement.style.colorScheme).toBe('dark'); + }); + + it('propagates a live theme toggle to open popups via the observer', async () => { + document.documentElement.setAttribute('data-theme', 'light'); + const { result } = renderHook(() => useDetachedTabs()); + act(() => { result.current.detach('n'); }); + const doc = fakePopups[0].document; + expect(doc.documentElement.getAttribute('data-theme')).toBe('light'); + + await act(async () => { + document.documentElement.setAttribute('data-theme', 'dark'); + // Let the MutationObserver microtask deliver. + await Promise.resolve(); + await Promise.resolve(); + }); + expect(doc.documentElement.getAttribute('data-theme')).toBe('dark'); + }); + }); }); diff --git a/frontend/src/hooks/useDetachedTabs.ts b/frontend/src/hooks/useDetachedTabs.ts index cdfa83bd..600da855 100644 --- a/frontend/src/hooks/useDetachedTabs.ts +++ b/frontend/src/hooks/useDetachedTabs.ts @@ -7,6 +7,24 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'; import type { TabId } from '../types'; +import { colors } from '../styles/tokens'; + +/** Resolve the opener's current theme ('dark' | 'light'). */ +function currentTheme(): 'dark' | 'light' { + return document.documentElement.getAttribute('data-theme') === 'dark' ? 'dark' : 'light'; +} + +/** + * Mirror the opener's theme onto a popup document (QW19). The popup clones the + * opener's stylesheets (incl. tokens.css with its `[data-theme="dark"]` + * overrides), but NOT the `data-theme` attribute the theme toggle writes on the + * opener's — so without this a detached tab always rendered light. + */ +function mirrorTheme(popupDoc: Document): void { + const theme = currentTheme(); + popupDoc.documentElement.setAttribute('data-theme', theme); + popupDoc.documentElement.style.colorScheme = theme; +} /** * A "detached" tab lives in a secondary browser window. The React component @@ -56,7 +74,7 @@ function cloneStylesIntoPopup(popupDoc: Document): void { bodyStyle.width = '100vw'; bodyStyle.display = 'flex'; bodyStyle.flexDirection = 'column'; - bodyStyle.background = 'white'; + bodyStyle.background = colors.surface; popupDoc.documentElement.style.height = '100%'; } @@ -67,7 +85,7 @@ function cloneStylesIntoPopup(popupDoc: Document): void { function buildMountNode(popupDoc: Document): HTMLElement { const mount = popupDoc.createElement('div'); mount.id = 'costudy4grid-detached-root'; - mount.style.cssText = 'flex: 1; position: relative; width: 100%; min-height: 0; background: white;'; + mount.style.cssText = `flex: 1; position: relative; width: 100%; min-height: 0; background: ${colors.surface};`; popupDoc.body.appendChild(mount); return mount; } @@ -164,6 +182,7 @@ export function useDetachedTabs( popup.document.write(`Co-Study4Grid — ${TAB_TITLES[tabId]}`); popup.document.close(); cloneStylesIntoPopup(popup.document); + mirrorTheme(popup.document); const mountNode = buildMountNode(popup.document); // When the popup is closed (by the user, by reload, or by our @@ -179,6 +198,28 @@ export function useDetachedTabs( return entry; }, [pruneTab]); + // Keep open popups in sync with live theme toggles (QW19): observe the + // opener's data-theme and mirror every change onto each popup + // document. Reads detachedRef so it needs no dep churn. + useEffect(() => { + const observer = new MutationObserver(() => { + const theme = currentTheme(); + for (const entry of Object.values(detachedRef.current)) { + if (entry && !entry.window.closed) { + try { + entry.window.document.documentElement.setAttribute('data-theme', theme); + entry.window.document.documentElement.style.colorScheme = theme; + } catch { /* popup navigating away */ } + } + } + }); + observer.observe(document.documentElement, { + attributes: true, + attributeFilter: ['data-theme'], + }); + return () => observer.disconnect(); + }, []); + const isDetached = useCallback((tabId: TabId) => { const entry = detachedRef.current[tabId]; return !!entry && !entry.window.closed; diff --git a/frontend/src/hooks/useDiagramHighlights.test.ts b/frontend/src/hooks/useDiagramHighlights.test.ts index 7d231289..3f7ec063 100644 --- a/frontend/src/hooks/useDiagramHighlights.test.ts +++ b/frontend/src/hooks/useDiagramHighlights.test.ts @@ -150,3 +150,51 @@ describe('useDiagramHighlights — halo call order matches product spec', () => expect(overloadOrder).toBeLessThan(actionOrder); }); }); + +// QW14: the highlight pipeline used to re-run on every pan/zoom settle because +// applyHighlightsForTab depended on the whole `diagrams` object, whose identity +// churns when the per-tab pan/zoom (nPZ/n1PZ/actionPZ) changes. Narrowing that +// dependency to the specific members the callback reads (the two container refs +// + two metaIndexes) makes a settle a no-op for highlighting. +describe('useDiagramHighlights — no re-run on pan/zoom settle (QW14)', () => { + const callCount = (fn: unknown) => + (fn as unknown as { mock: { calls: unknown[] } }).mock.calls.length; + + beforeEach(() => vi.clearAllMocks()); + + it('does not re-invoke the highlight helpers when only the diagrams object identity changes', () => { + const diagrams = makeDiagramsState({ activeTab: 'contingency' }); + const result = { + lines_overloaded: ['OVL_LINE'], actions: {}, action_scores: {}, + } as unknown as AnalysisResult; + // Hoisted so ONLY the `diagrams` object identity changes on rerender — + // otherwise a fresh Set / {} each render would trip other effect deps. + const selectedOverloads = new Set(); + const detachedTabs = {}; + const props = (d: DiagramsState) => ({ + diagrams: d, + result, + selectedBranch: 'CONT_LINE', + selectedOverloads, + monitoringFactor: 0.95, + detachedTabs, + }); + + const { rerender } = renderHook( + (p: ReturnType) => useDiagramHighlights(p), + { initialProps: props(diagrams) }, + ); + // Initial pass ran the pipeline. + expect(callCount(applyContingencyHighlight)).toBeGreaterThan(0); + vi.clearAllMocks(); + + // Simulate a pan/zoom settle: a NEW `diagrams` object identity, but every + // member (container refs, metaIndexes, the three diagrams) is the SAME + // reference. With the narrowed deps this must not re-run the pipeline. + rerender(props({ ...diagrams })); + + expect(callCount(applyContingencyHighlight)).toBe(0); + expect(callCount(applyOverloadedHighlights)).toBe(0); + expect(callCount(applyDeltaVisuals)).toBe(0); + }); +}); diff --git a/frontend/src/hooks/useDiagramHighlights.ts b/frontend/src/hooks/useDiagramHighlights.ts index 36e3b3f1..841f6a0e 100644 --- a/frontend/src/hooks/useDiagramHighlights.ts +++ b/frontend/src/hooks/useDiagramHighlights.ts @@ -246,7 +246,18 @@ export function useDiagramHighlights({ // layer. applyDeltaVisuals(diagrams.actionSvgContainerRef.current, actionDiagram, diagrams.actionMetaIndex, effectiveMode === 'delta'); } - }, [n1Diagram, actionDiagram, result, selectedActionId, selectedContingency, diagrams, monitoringFactor, viewModeForTab, selectedOverloads]); + // QW14: depend on the SPECIFIC `diagrams` members this callback reads, not + // the whole `diagrams` object — its identity churns on every pan/zoom + // settle (nPZ/n1PZ/actionPZ change), which re-created this callback and + // re-fired the driving effect below, re-running the whole highlight + // pipeline on every gesture end. The two *SvgContainerRef values are stable + // ref objects and the two *MetaIndex values are memoized off diagram + // metadata, so no legitimate trigger is lost (the driving effect also lists + // nDiagram/n1Diagram/actionDiagram + the metaIndexes directly). + }, [n1Diagram, actionDiagram, result, selectedActionId, selectedContingency, + diagrams.n1SvgContainerRef, diagrams.actionSvgContainerRef, + diagrams.n1MetaIndex, diagrams.actionMetaIndex, + monitoringFactor, viewModeForTab, selectedOverloads]); useEffect(() => { const isTabSwitch = prevHighlightTabRef.current !== activeTab; diff --git a/frontend/src/hooks/useModalKeyboard.test.tsx b/frontend/src/hooks/useModalKeyboard.test.tsx new file mode 100644 index 00000000..4256e7f1 --- /dev/null +++ b/frontend/src/hooks/useModalKeyboard.test.tsx @@ -0,0 +1,87 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom/vitest'; +import { useModalKeyboard } from './useModalKeyboard'; + +function Modal({ isOpen, onClose, closeOnEscape }: { isOpen: boolean; onClose: () => void; closeOnEscape?: boolean }) { + const { containerRef, dialogProps } = useModalKeyboard({ isOpen, onClose, closeOnEscape }); + if (!isOpen) return null; + return ( +
+ + +
+ ); +} + +describe('useModalKeyboard (QW20)', () => { + it('stamps role="dialog" + aria-modal on the container', () => { + render(); + const modal = screen.getByTestId('modal'); + expect(modal).toHaveAttribute('role', 'dialog'); + expect(modal).toHaveAttribute('aria-modal', 'true'); + }); + + it('calls onClose on Escape when open', () => { + const onClose = vi.fn(); + render(); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not call onClose on Escape when closeOnEscape is false', () => { + const onClose = vi.fn(); + render(); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('does nothing on Escape when closed', () => { + const onClose = vi.fn(); + render(); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(onClose).not.toHaveBeenCalled(); + }); + + it('moves focus into the modal on open', () => { + render(); + // The container (tabIndex=-1) receives focus on open. + expect(screen.getByTestId('modal')).toHaveFocus(); + }); + + it('restores focus to the trigger element on close', () => { + const trigger = document.createElement('button'); + document.body.appendChild(trigger); + trigger.focus(); + expect(trigger).toHaveFocus(); + + const { rerender } = render(); + expect(screen.getByTestId('modal')).toHaveFocus(); + + rerender(); + expect(trigger).toHaveFocus(); + document.body.removeChild(trigger); + }); + + it('wraps Tab from last focusable back to first (focus trap)', () => { + render(); + const last = screen.getByTestId('last'); + last.focus(); + fireEvent.keyDown(document, { key: 'Tab' }); + expect(screen.getByTestId('first')).toHaveFocus(); + }); + + it('wraps Shift+Tab from first focusable to last', () => { + render(); + const first = screen.getByTestId('first'); + first.focus(); + fireEvent.keyDown(document, { key: 'Tab', shiftKey: true }); + expect(screen.getByTestId('last')).toHaveFocus(); + }); +}); diff --git a/frontend/src/hooks/useModalKeyboard.ts b/frontend/src/hooks/useModalKeyboard.ts new file mode 100644 index 00000000..bd19ef28 --- /dev/null +++ b/frontend/src/hooks/useModalKeyboard.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +// This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +// If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +// you can obtain one at http://mozilla.org/MPL/2.0/. +// SPDX-License-Identifier: MPL-2.0 + +// Shared modal keyboard / focus behaviour (QW20). The dialogs previously had +// no Escape-to-close, no initial focus, no focus trap, and inconsistent +// `role`/`aria-modal`. This hook centralises all of it: +// - Escape closes the modal (opt-out via closeOnEscape). +// - On open, focus moves into the modal (initialFocusRef ?? the container). +// - Tab / Shift+Tab wrap within the modal's focusable descendants (trap). +// - On close, focus is restored to whatever had it before the modal opened. +// - `dialogProps` stamps role="dialog" + aria-modal + tabIndex=-1. + +import { useEffect, useRef, type RefObject } from 'react'; + +const FOCUSABLE_SELECTOR = [ + 'a[href]', + 'button:not([disabled])', + 'textarea:not([disabled])', + 'input:not([disabled])', + 'select:not([disabled])', + '[tabindex]:not([tabindex="-1"])', +].join(', '); + +export interface ModalKeyboardOptions { + isOpen: boolean; + onClose: () => void; + /** Default true — set false for modals that must not close on Escape. */ + closeOnEscape?: boolean; + /** Element to focus on open; falls back to the modal container. */ + initialFocusRef?: RefObject; +} + +export interface ModalKeyboardResult { + containerRef: RefObject; + dialogProps: { readonly role: 'dialog'; readonly 'aria-modal': true; readonly tabIndex: -1 }; +} + +const DIALOG_PROPS = { role: 'dialog', 'aria-modal': true, tabIndex: -1 } as const; + +export function useModalKeyboard( + { isOpen, onClose, closeOnEscape = true, initialFocusRef }: ModalKeyboardOptions, +): ModalKeyboardResult { + const containerRef = useRef(null); + const restoreFocusRef = useRef(null); + // Latest-ref for the caller-provided values so the keydown effect below + // depends only on `isOpen` / `closeOnEscape`. Without this, an inline + // `onClose` (a new identity each render) would re-run the effect on every + // render — and its cleanup restores focus, so the modal would thrash focus. + const onCloseRef = useRef(onClose); + const initialFocusStore = useRef(initialFocusRef); + useEffect(() => { + onCloseRef.current = onClose; + initialFocusStore.current = initialFocusRef; + }); + + useEffect(() => { + if (!isOpen) return; + // Remember what had focus so we can restore it on close. + restoreFocusRef.current = document.activeElement as HTMLElement | null; + (initialFocusStore.current?.current ?? containerRef.current)?.focus(); + + const onKeyDown = (e: KeyboardEvent) => { + if (closeOnEscape && e.key === 'Escape') { + e.stopPropagation(); + onCloseRef.current(); + return; + } + if (e.key !== 'Tab' || !containerRef.current) return; + // The selector already excludes disabled controls and + // tabindex="-1"; DOM order is what the wrap needs. (No visibility + // filter — offsetParent/getClientRects are unavailable under jsdom + // and display:none controls aren't Tab-reachable anyway.) + const focusables = Array.from( + containerRef.current.querySelectorAll(FOCUSABLE_SELECTOR), + ); + if (focusables.length === 0) { + e.preventDefault(); + containerRef.current.focus(); + return; + } + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + const active = document.activeElement; + if (e.shiftKey && (active === first || active === containerRef.current)) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && active === last) { + e.preventDefault(); + first.focus(); + } + }; + + document.addEventListener('keydown', onKeyDown, true); + return () => { + document.removeEventListener('keydown', onKeyDown, true); + restoreFocusRef.current?.focus?.(); + }; + }, [isOpen, closeOnEscape]); + + return { containerRef, dialogProps: DIALOG_PROPS }; +} From f5634e1fc93e99c230762faee5277e0760ede795 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 14:00:07 +0000 Subject: [PATCH 20/24] feat(qw): error-leak hardening (QW6), recommender pin + canary (QW8), single CI (QW23), Game-Mode resilience (QW24), Dockerfile hygiene (QW25) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QW6: genericise the seven path-leaking FS/config str(e) error sites in main.py (save/set config, /api/config, pick-path, save/list/load-session) — each now logs server-side via logger.exception and returns a generic detail, so absolute paths / raw exception text no longer reach the client. D2 only covered UNCAUGHT exceptions; these raise explicit HTTPException(400)s. +2 leak-guard tests. QW8: pin expert_op4grid_recommender via recommender-pin.txt (one exact version) for PR + Docker installs, so an upstream release can't silently break a PR or a rebuild; new weekly canary.yml floats to latest and flags regressions. Consistency guard test scripts/test_recommender_pin_consistency.py. requirements_py310.txt un-stalened (0.1.1 -> 0.3.0.post1). QW23: delete .circleci/config.yml (GitHub Actions is a strict superset); scrub the CircleCI references from CLAUDE.md / CONTRIBUTING.md / code-quality-analysis. QW24: Game Mode now recovers from a mid-session backend failure — retryStudy re-attempts the failed study and finishEarly ends with the already-committed results instead of destroying them; GameShell surfaces Retry / Finish / Quit. + a preset↔overload-data consistency test (every preset contingency exists in its grid's n1_overload_contingencies.json). +3 tests. QW25: Dockerfile hygiene — honor $PORT (was decorative; shell-form CMD), HEALTHCHECK on the always-available /api/user-config, and a committed+tested scripts/extract_network_zip.py that FAILS LOUDLY on an un-smudged Git-LFS pointer / corrupt archive instead of baking a broken grid; .dockerignore drops the test suites from the image. +4 extract-validator tests. (The jupyter-util runtime-dep slim is deferred — it touches nad/sld render imports.) Gate + tsc + eslint clean; backend + frontend + scripts suites green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- .circleci/config.yml | 99 --------------------- .dockerignore | 5 ++ .github/workflows/canary.yml | 34 +++++++ .github/workflows/test.yml | 33 +++---- CLAUDE.md | 6 +- CONTRIBUTING.md | 7 +- Dockerfile | 29 ++++-- docs/architecture/code-quality-analysis.md | 17 ++-- expert_backend/main.py | 36 ++++---- expert_backend/tests/test_api_endpoints.py | 32 +++++++ frontend/src/game/GameShell.tsx | 46 +++++++--- frontend/src/game/presets.test.ts | 30 +++++++ frontend/src/game/useGameSession.test.ts | 39 ++++++++ frontend/src/game/useGameSession.ts | 32 +++++++ recommender-pin.txt | 9 ++ requirements_py310.txt | 2 +- scripts/extract_network_zip.py | 52 +++++++++++ scripts/test_extract_network_zip.py | 46 ++++++++++ scripts/test_recommender_pin_consistency.py | 55 ++++++++++++ 19 files changed, 446 insertions(+), 163 deletions(-) delete mode 100644 .circleci/config.yml create mode 100644 .github/workflows/canary.yml create mode 100644 recommender-pin.txt create mode 100644 scripts/extract_network_zip.py create mode 100644 scripts/test_extract_network_zip.py create mode 100644 scripts/test_recommender_pin_consistency.py diff --git a/.circleci/config.yml b/.circleci/config.yml deleted file mode 100644 index a033cb16..00000000 --- a/.circleci/config.yml +++ /dev/null @@ -1,99 +0,0 @@ -version: 2.1 - -orbs: - python: circleci/python@2.1.1 - node: circleci/node@5.1.0 - -jobs: - test-backend: - docker: - - image: cimg/python:3.10 - steps: - - checkout - - run: - name: Install Graphviz (provides ``dot``) - command: sudo apt-get update && sudo apt-get install -y graphviz - - run: - name: Install Dependencies - command: | - python -m pip install --upgrade pip - pip install ".[test]" - # Track the latest published expert_op4grid_recommender release: the - # >=0.2.4 floor documents the minimum the GST / superposition tests - # need, and pip + --no-cache-dir always resolve to the newest release - # above it (no stale pinned version to bump). --no-deps skips the - # recommender's heavy transitive stack (pypowsybl already comes from - # co_study4grid's own dependencies). - pip install --no-deps --no-cache-dir "expert_op4grid_recommender>=0.2.4" - - run: - name: Run Pytest - # Coverage GATES at fail_under=72 (pyproject [tool.coverage.report]). - # See docs/architecture/code-quality-analysis.md §20. - command: pytest --cov=expert_backend --cov-report=xml --cov-report=term-missing - - store_artifacts: - path: coverage.xml - destination: backend-coverage - - test-frontend: - docker: - - image: cimg/node:20.10 - steps: - - checkout - - run: - name: Install Frontend Dependencies - command: | - cd frontend - npm install - - run: - name: Run Frontend Tests + coverage gate - command: | - cd frontend - npm run test:coverage - - store_artifacts: - path: frontend/coverage - destination: frontend-coverage - - run: - name: Run Linter - command: | - cd frontend - npm run lint - - code-quality: - docker: - - image: cimg/python:3.10 - steps: - - checkout - - run: - name: Install quality tooling - command: | - python -m pip install --upgrade pip - pip install ".[quality]" - pip install pytest - - run: - name: Ruff (lightweight lint) - command: ruff check expert_backend scripts - - run: - name: Code-quality gate - command: python scripts/check_code_quality.py - - run: - name: Reporter unit tests - command: pytest scripts/test_code_quality_report.py -q - - run: - name: mypy (gate) - command: mypy - - run: - name: Generate report artifacts - command: | - python scripts/code_quality_report.py \ - --output reports/code-quality.json \ - --markdown reports/code-quality.md - - store_artifacts: - path: reports - destination: code-quality - -workflows: - test-workflow: - jobs: - - test-backend - - test-frontend - - code-quality diff --git a/.dockerignore b/.dockerignore index 009cd1fc..0b96fc80 100644 --- a/.dockerignore +++ b/.dockerignore @@ -26,3 +26,8 @@ test-results # Docs and the frozen legacy mirror are not needed at runtime. docs standalone_interface_legacy.html + +# Test suites are dead weight in the runtime image (QW25) — the app never +# imports them, and `COPY expert_backend/` would otherwise carry tests/ along. +expert_backend/tests +expert_backend/test_backend.py diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml new file mode 100644 index 00000000..7585dc2a --- /dev/null +++ b/.github/workflows/canary.yml @@ -0,0 +1,34 @@ +name: Recommender canary + +# QW8: PR/deploy installs pin expert_op4grid_recommender (recommender-pin.txt). +# This weekly canary floats to the LATEST release and runs the backend suite, +# so a breaking upstream release surfaces here (a red canary = "bump the pin +# after checking") instead of turning an unrelated PR red or shipping a +# regressed image on a zero-change rebuild. + +on: + schedule: + - cron: '0 6 * * 1' # Mondays 06:00 UTC + workflow_dispatch: {} + +jobs: + canary: + name: Backend tests against latest recommender + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up Python 3.10 + uses: actions/setup-python@v5 + with: + python-version: "3.10" + cache: 'pip' + - name: Install dependencies (float recommender to latest) + run: | + python -m pip install --upgrade pip + pip install ".[test]" + # Deliberately NOT pinned: resolve the newest published release. + pip install --no-deps --no-cache-dir --upgrade "expert_op4grid_recommender" + - name: Show resolved recommender version + run: pip show expert_op4grid_recommender | grep -i version + - name: Run backend Pytest (excluding graphviz-dependent tests) + run: pytest --ignore=expert_backend/tests/test_overflow_html_dim_logic.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 36aaabee..bbbc5f07 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,13 +29,12 @@ jobs: run: | python -m pip install --upgrade pip pip install ".[test]" - # Track the latest published expert_op4grid_recommender release: the - # >=0.2.4 floor documents the minimum the GST / superposition tests - # need, and pip + --no-cache-dir always resolve to the newest release - # above it (no stale pinned version to bump). --no-deps skips the - # recommender's heavy transitive stack (pypowsybl already comes from - # co_study4grid's own dependencies). - pip install --no-deps --no-cache-dir "expert_op4grid_recommender>=0.2.4" + # Pin expert_op4grid_recommender to the exact version in + # recommender-pin.txt (QW8) so an upstream release can't silently turn + # a green PR red; the weekly canary.yml floats to latest and flags + # regressions. --no-deps skips the recommender's heavy transitive + # stack (pypowsybl already comes from co_study4grid's own deps). + pip install --no-deps -r recommender-pin.txt - name: Run Pytest (excluding graphviz-dependent tests) # Coverage GATES at fail_under=72 (pyproject [tool.coverage.report]). # See docs/architecture/code-quality-analysis.md §20. @@ -76,8 +75,11 @@ jobs: run: | python -m pip install --upgrade pip pip install ".[test]" - - name: Run pipeline + scorer tests (hermetic slice) - run: pytest scripts/pypsa_eur scripts/game_mode -q + - name: Run pipeline + scorer + config-consistency tests (hermetic slice) + run: >- + pytest scripts/pypsa_eur scripts/game_mode + scripts/test_recommender_pin_consistency.py + scripts/test_extract_network_zip.py -q test-frontend: runs-on: ubuntu-latest @@ -151,12 +153,11 @@ jobs: run: | python -m pip install --upgrade pip pip install ".[test]" - # Track the latest published expert_op4grid_recommender release: the - # >=0.2.4 floor documents the minimum the GST / superposition tests - # need, and pip + --no-cache-dir always resolve to the newest release - # above it (no stale pinned version to bump). --no-deps skips the - # recommender's heavy transitive stack (pypowsybl already comes from - # co_study4grid's own dependencies). - pip install --no-deps --no-cache-dir "expert_op4grid_recommender>=0.2.4" + # Pin expert_op4grid_recommender to the exact version in + # recommender-pin.txt (QW8) so an upstream release can't silently turn + # a green PR red; the weekly canary.yml floats to latest and flags + # regressions. --no-deps skips the recommender's heavy transitive + # stack (pypowsybl already comes from co_study4grid's own deps). + pip install --no-deps -r recommender-pin.txt - name: Run graphviz-dependent Pytest subset run: pytest expert_backend/tests/test_overflow_html_dim_logic.py diff --git a/CLAUDE.md b/CLAUDE.md index 13c183e6..3dcd619b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -286,8 +286,8 @@ python scripts/code_quality_report.py --output reports/code-quality.json \ python scripts/check_code_quality.py ``` -Both scripts run in CI (`.github/workflows/code-quality.yml` and -`.circleci/config.yml`). The gate guards the reductions documented in +Both scripts run in CI (`.github/workflows/code-quality.yml`). The gate +guards the reductions documented in [`docs/architecture/code-quality-analysis.md`](docs/architecture/code-quality-analysis.md) (no new `print()` / bare except, module-size ceilings, no `any` / `@ts-ignore` in frontend source). @@ -443,7 +443,7 @@ NAD/SLD payloads: - **Deployment lockdown (D7, 2026-07)**: the `Dockerfile` sets `COSTUDY4GRID_LOCKDOWN=1`, which disables the desktop-era filesystem RPCs (custom config-file path, session save/list/load, native file picker) with a `403 {code: LOCKED_DOWN}` — they assume a local operator and would otherwise expose the container filesystem to an anonymous Space visitor. The read-only app config stays available so the SPA boots; unset locally so dev is unaffected. The HF deploy is **test-gated** (`workflow_run` on a green Tests run) and **tags each deploy** on origin (`space-deploy-*`) as the rollback pointer. See [`docs/architecture/deployment-trust.md`](docs/architecture/deployment-trust.md). - **Game Mode (0.8.0)**: a timed, scored session shell in `frontend/src/game/`, **additive and inert unless `?game=1`** — `main.tsx` mounts `GameShell` instead of `App`; App integration is three `gameBridge.isGameMode()`-guarded touch points. See `docs/features/game-mode-codabench.md`. - **Binary assets via Git LFS + transparent network decompression (0.8.0)**: `.gitattributes` tracks `*.zip` / `*.png` / `*.jpg` via Git LFS (the HuggingFace Space git endpoint rejects non-LFS binaries); the large France 225/400 kV grid ships as `network.xiidm.zip` and `network_service._resolve_network_file` / `_extract_network_zip` decompress it transparently on load. -- **CI pipelines**: GitHub Actions (`.github/workflows/code-quality.yml`, `parity.yml`, `test.yml`) and CircleCI (`.circleci/config.yml`) run the code-quality gate, ruff, the pytest + Vitest suites, and the parity scripts. The backend test installs **always track the latest `expert_op4grid_recommender`** (`>=0.2.4` floor + `--no-cache-dir`, so a fresh index resolves to the newest release); the `Dockerfile` pins the same floor to keep the deployed recommender consistent with CI. +- **CI pipelines**: GitHub Actions only (`.github/workflows/`: `code-quality.yml`, `parity.yml`, `test.yml`, `canary.yml`, `deploy-huggingface.yml`) run the code-quality gate, ruff, the pytest + Vitest suites, the data-pipeline + scorer-parity slice, and the parity scripts. CircleCI was removed (QW23 — GitHub Actions is a strict superset). The backend test + the `Dockerfile` install `expert_op4grid_recommender` from **`recommender-pin.txt`** — one pinned, exact version (QW8) so an upstream release can't silently break a PR or a rebuild; the weekly `canary.yml` floats to the latest release and flags regressions, so upgrades are a deliberate bump of that file. - Root `.gitignore` excludes `__pycache__/`, `*.pyc`, `*.pyo`; `frontend/.gitignore` handles frontend build artifacts - Integration helpers and parity scripts live under `scripts/`. They are NOT part of the pytest suite — invoke them directly. The PyPSA-EUR pipeline scripts under `scripts/pypsa_eur/` DO carry pytest coverage (`test_build_pipeline.py`, `test_calibrate_thermal_limits.py`, `test_generate_n1_overloads.py`, `test_regenerate_grid_layout.py`). - `overrides.txt` contains pinned versions for transitive Python dependencies that need to be forced to specific versions diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 80fe646f..4289c7b5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,8 +70,7 @@ npm run lint Continuous quality metrics are generated by [`scripts/code_quality_report.py`](./scripts/code_quality_report.py) and enforced by [`scripts/check_code_quality.py`](./scripts/check_code_quality.py). -Both run locally and in CI (see `.github/workflows/code-quality.yml` and -`.circleci/config.yml`). +Both run locally and in CI (see `.github/workflows/code-quality.yml`). ```bash # Generate a full JSON + Markdown report @@ -121,8 +120,8 @@ via `frontend/vite.config.ts` (`coverage.thresholds`, enforced by `npm run test:coverage`) and backend via `pyproject.toml` (`[tool.coverage.report] fail_under = 72`, enforced by `pytest --cov`). Both floors sit a few points below the measured baseline — raise them as -coverage climbs, don't lower them. All are wired into the GitHub Actions + -CircleCI pipelines; see §§19–20 of the analysis doc. +coverage climbs, don't lower them. All are wired into the GitHub Actions +pipelines; see §§19–20 of the analysis doc. ## Commit & PR conventions diff --git a/Dockerfile b/Dockerfile index bf38446a..681b95cd 100644 --- a/Dockerfile +++ b/Dockerfile @@ -50,12 +50,14 @@ WORKDIR /home/user/app # fastapi, …). The recommender ships with `--no-deps` — mirroring CI — because # its own dependency tree is self-conflicting (it wants numpy>=2 while its # transitive `pypowsybl2grid` pins numpy==1.26.4); the working runtime deps -# come from `pip install .`. overrides.txt forces the pinned versions last. -COPY --chown=user pyproject.toml README.md overrides.txt ./ +# come from `pip install .`. It is pinned via recommender-pin.txt (QW8) — the +# SAME file CI installs — so a zero-change rebuild can't drift to a new +# release. overrides.txt forces the pinned transitive versions last. +COPY --chown=user pyproject.toml README.md overrides.txt recommender-pin.txt ./ COPY --chown=user expert_backend/ ./expert_backend/ RUN pip install --no-cache-dir --upgrade pip \ && pip install --no-cache-dir . \ - && pip install --no-cache-dir --no-deps "expert_op4grid_recommender>=0.2.4" \ + && pip install --no-cache-dir --no-deps -r recommender-pin.txt \ && pip install --no-cache-dir -r overrides.txt # --- Application code, bundled grids, built SPA ---------------------------- @@ -63,11 +65,13 @@ RUN pip install --no-cache-dir --upgrade pip \ # the backend copies it to config.json on first boot. COPY --chown=user config.default.json ./ COPY --chown=user data/ ./data/ -# The European grid ships compressed (its raw .xiidm exceeds HuggingFace's -# 10 MiB git file limit, so it travels as a Git-LFS .zip). Decompress it here -# so pypowsybl can load network.xiidm directly — the "Medium" game difficulty. -RUN python -c "import zipfile, pathlib; z = pathlib.Path('data/pypsa_eur_eur220_225_380_400/network.xiidm.zip'); zipfile.ZipFile(z).extractall(z.parent) if z.exists() else print('eur220 network zip absent — skipping')" COPY --chown=user scripts/ ./scripts/ +# The European grid ships compressed (its raw .xiidm exceeds HuggingFace's +# 10 MiB git file limit, so it travels as a Git-LFS .zip). Validate + decompress +# it so pypowsybl can load network.xiidm directly — the "Medium" game difficulty. +# The helper FAILS LOUDLY on an un-smudged LFS pointer or a corrupt archive +# (QW25) instead of silently baking a broken grid into the image. +RUN python scripts/extract_network_zip.py data/pypsa_eur_eur220_225_380_400/network.xiidm.zip COPY --chown=user --from=frontend /build/dist ./frontend/dist # The overflow-viewer overlay (services/overflow_overlay.py) inlines this # shared pin-glyph source module at request time, so it must exist at the @@ -92,4 +96,13 @@ ENV COSTUDY4GRID_FRONTEND_DIST=/home/user/app/frontend/dist \ # lockdown profile in expert_backend/main.py. EXPOSE 7860 -CMD ["uvicorn", "expert_backend.main:app", "--host", "0.0.0.0", "--port", "7860"] + +# Liveness probe (QW25): the read-only app-config endpoint stays available even +# under lockdown (D7), so it's a safe heartbeat. The long start-period covers +# the slow first network load. Uses $PORT so it tracks a non-default port. +HEALTHCHECK --interval=30s --timeout=5s --start-period=120s --retries=3 \ + CMD python -c "import os,sys,urllib.request; urllib.request.urlopen('http://127.0.0.1:%s/api/user-config' % os.environ.get('PORT','7860')).read(); sys.exit(0)" + +# Shell form so ${PORT} expands (QW25: PORT was decorative before); `exec` +# keeps uvicorn as PID 1 so it receives SIGTERM directly for clean shutdown. +CMD ["sh", "-c", "exec uvicorn expert_backend.main:app --host 0.0.0.0 --port ${PORT:-7860}"] diff --git a/docs/architecture/code-quality-analysis.md b/docs/architecture/code-quality-analysis.md index 0e0cacfc..0a4fd058 100644 --- a/docs/architecture/code-quality-analysis.md +++ b/docs/architecture/code-quality-analysis.md @@ -8,7 +8,7 @@ > - `python scripts/code_quality_report.py` — metrics dump (JSON + Markdown) > - `python scripts/check_code_quality.py` — CI gate (non-zero exit on regression) > - Unit-tested in `scripts/test_code_quality_report.py` (13 tests, part of the pytest suite) -> - Runs in CI: `.github/workflows/code-quality.yml` + `.circleci/config.yml` +> - Runs in CI: `.github/workflows/code-quality.yml` (CircleCI removed — QW23) > (jobs `gate` / `code-quality`). Also uploads a report artifact. > - See [CONTRIBUTING.md](../../CONTRIBUTING.md#code-quality-checks) > for local usage and thresholds. @@ -1049,12 +1049,15 @@ widening hot files: ### 16.4 CI / supply-chain note -From 0.8.0 the backend test installs **track the latest published -``expert_op4grid_recommender``** (``>=0.2.4`` floor + ``--no-cache-dir``) -rather than a pinned version, so a fresh index resolves to the newest -release on every run (``.github/workflows/test.yml`` / -``.circleci/config.yml``); the ``Dockerfile`` pins the same floor to -keep the deployed recommender consistent with CI. The new HuggingFace +From 0.8.0 the backend test installs tracked the latest published +``expert_op4grid_recommender`` on every run — which meant an upstream +release could turn an unrelated PR red or ship a regressed image on a +zero-change rebuild. **From 0.9.0 (QW8) the PR/deploy installs pin an +exact version** via ``recommender-pin.txt`` (consumed by +``.github/workflows/test.yml`` and the ``Dockerfile``); a weekly +``canary.yml`` floats to the latest release so upgrades are a deliberate +bump of that file. CircleCI was removed in the same release (QW23 — +GitHub Actions is a strict superset). The new HuggingFace Docker deployment adds an **optional** same-origin SPA mount in ``main.py`` (``COSTUDY4GRID_FRONTEND_DIST``, mounted last, inert when absent — local dev / the test suite are unaffected) and Git LFS diff --git a/expert_backend/main.py b/expert_backend/main.py index f806feab..90f7a5ea 100644 --- a/expert_backend/main.py +++ b/expert_backend/main.py @@ -262,8 +262,9 @@ def save_user_config(config: dict = Body(...)) -> dict: try: _save_user_config(config) return {"status": "success"} - except Exception as e: - raise HTTPException(status_code=400, detail=str(e)) + except Exception: + logger.exception("Failed to save user config") + raise HTTPException(status_code=400, detail="Failed to save configuration.") @app.get("/api/config-file-path") @@ -283,8 +284,9 @@ def set_config_file_path(path: str = Body(..., embed=True)) -> dict: return {"status": "success", "config_file_path": str(new_path), "config": _load_user_config()} except HTTPException: raise - except Exception as e: - raise HTTPException(status_code=400, detail=str(e)) + except Exception: + logger.exception("Failed to set config file path") + raise HTTPException(status_code=400, detail="Failed to set the config file path.") class ConfigRequest(BaseModel): @@ -477,8 +479,9 @@ def update_config(config: ConfigRequest) -> dict: "active_model": recommender_service.get_active_model_name(), "compute_overflow_graph": recommender_service.get_compute_overflow_graph(), } - except Exception as e: - raise HTTPException(status_code=400, detail=str(e)) + except Exception: + logger.exception("Config load failed") + raise HTTPException(status_code=400, detail="Failed to load the network configuration.") finally: recommender_service.end_study_mutation() @@ -555,9 +558,9 @@ def pick_path(type: str = Query("file", enum=["file", "dir"])) -> dict: return _pick_path_tkinter(type) except subprocess.TimeoutExpired: return {"path": "", "error": "File picker timed out (no selection made)."} - except Exception as e: - logger.warning("Error picking path: %s", e) - return {"path": "", "error": str(e)} + except Exception: + logger.exception("Error picking path") + return {"path": "", "error": "File picker failed."} def _pick_path_macos(kind: str) -> dict: @@ -648,8 +651,9 @@ def save_session(request: SaveSessionRequest) -> dict: session_dir = _safe_session_dir(request.output_folder_path, request.session_name) try: os.makedirs(session_dir, exist_ok=True) - except OSError as e: - raise HTTPException(status_code=400, detail=f"Cannot create session directory: {e}") + except OSError: + logger.exception("Cannot create session directory") + raise HTTPException(status_code=400, detail="Cannot create the session directory.") json_file = os.path.join(session_dir, "session.json") with open(json_file, "w", encoding="utf-8") as f: @@ -691,8 +695,9 @@ def list_sessions(folder_path: str = Query(...)) -> dict: json_path = os.path.join(entry_path, "session.json") if os.path.isfile(json_path): sessions.append(entry) - except OSError as e: - raise HTTPException(status_code=400, detail=f"Cannot read folder: {e}") + except OSError: + logger.exception("Cannot read sessions folder") + raise HTTPException(status_code=400, detail="Cannot read the sessions folder.") sessions.sort(reverse=True) return {"sessions": sessions} @@ -732,8 +737,9 @@ def load_session(folder_path: str = Body(...), session_name: str = Body(...)) -> shutil.copy2(picked, target_path) return content - except Exception as e: - raise HTTPException(status_code=400, detail=f"Failed to read session: {e}") + except Exception: + logger.exception("Failed to read session") + raise HTTPException(status_code=400, detail="Failed to read the session file.") @app.post("/api/restore-analysis-context", response_model=RestoreAnalysisContextResponse) def restore_analysis_context(request: RestoreAnalysisContextRequest) -> dict: diff --git a/expert_backend/tests/test_api_endpoints.py b/expert_backend/tests/test_api_endpoints.py index f2bb9c20..08555b9c 100644 --- a/expert_backend/tests/test_api_endpoints.py +++ b/expert_backend/tests/test_api_endpoints.py @@ -1953,3 +1953,35 @@ def test_non_macos_uses_tkinter_subprocess(self, client): assert argv[0] != "osascript" # The inlined script must contain the tkinter import. assert any("import tkinter" in a for a in argv) + + +class TestErrorLeakGuard: + """QW6 — the FS/config endpoints must not leak filesystem paths (or raw + exception text) into client-visible error `detail`. D2 genericises only + UNCAUGHT exceptions; these endpoints raise explicit HTTPException(400)s, so + they need their own generic messages + server-side logger.exception.""" + + def test_save_user_config_failure_is_generic(self, client, monkeypatch): + def boom(_cfg): + raise OSError("/secret/abs/path/config.json: permission denied") + + monkeypatch.setattr("expert_backend.main._save_user_config", boom) + r = client.post("/api/user-config", json={"foo": "bar"}) + assert r.status_code == 400 + detail = r.json()["detail"] + assert detail == "Failed to save configuration." + assert "/secret" not in detail + + def test_save_session_failure_is_generic(self, client, monkeypatch): + # os.makedirs failing must not echo the absolute session path. + def boom(*_a, **_k): + raise OSError("/private/sessions/leak: no space left") + + monkeypatch.setattr("expert_backend.main.os.makedirs", boom) + r = client.post("/api/save-session", json={ + "output_folder_path": "out", "session_name": "s1", "json_content": "{}", + }) + assert r.status_code == 400 + detail = r.json()["detail"] + assert detail == "Cannot create the session directory." + assert "/private" not in detail diff --git a/frontend/src/game/GameShell.tsx b/frontend/src/game/GameShell.tsx index 524a18d7..63fe3172 100644 --- a/frontend/src/game/GameShell.tsx +++ b/frontend/src/game/GameShell.tsx @@ -78,16 +78,42 @@ export default function GameShell() {
{game.loadError}
- +
+ + + +
) : ( <> diff --git a/frontend/src/game/presets.test.ts b/frontend/src/game/presets.test.ts index e61dd4cb..a876716a 100644 --- a/frontend/src/game/presets.test.ts +++ b/frontend/src/game/presets.test.ts @@ -5,6 +5,8 @@ // SPDX-License-Identifier: MPL-2.0 import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; import { DIFFICULTY_TIERS, DEFAULT_DIFFICULTY, @@ -49,3 +51,31 @@ describe('game difficulty presets', () => { } }); }); + +// QW24b: the presets hardcode contingency ids; this guards them against the +// data artifacts they reference, so a regenerated / renamed grid that drops a +// contingency fails HERE (a fast static check) instead of silently shipping an +// unplayable Game Mode study. Complements the real-backend `can_proceed` replay +// in scripts/game_mode/e2e_game_session.py. +describe('preset ↔ overload-data consistency', () => { + interface OverloadData { contingencies: { tripped_line: string }[] } + + const trippedLineSet = (networkPath: string): Set => { + const jsonPath = resolve( + __dirname, '../../../', + networkPath.replace('network.xiidm', 'n1_overload_contingencies.json'), + ); + const data = JSON.parse(readFileSync(jsonPath, 'utf-8')) as OverloadData; + return new Set(data.contingencies.map(c => c.tripped_line)); + }; + + it.each(DIFFICULTY_TIERS)('every "$id" preset contingency exists in the grid overload data', (tier) => { + const trippedLines = trippedLineSet(tier.networkPath); + for (const study of tier.studies) { + expect( + trippedLines.has(study.contingencyElementId), + `preset "${study.id}" contingency ${study.contingencyElementId} is not in ${tier.networkPath}'s n1_overload_contingencies.json`, + ).toBe(true); + } + }); +}); diff --git a/frontend/src/game/useGameSession.test.ts b/frontend/src/game/useGameSession.test.ts index 35833807..8c8a611b 100644 --- a/frontend/src/game/useGameSession.test.ts +++ b/frontend/src/game/useGameSession.test.ts @@ -100,4 +100,43 @@ describe('useGameSession', () => { expect(log.studies.map((s) => s.solved)).toEqual([true, false, true]); expect(log.config.nStudies).toBe(3); }); + + // QW24: mid-session load failure must be recoverable, and must not destroy + // the results already committed. + it('retryStudy re-attempts a study that failed to load', async () => { + const loadSpy = vi.spyOn(gameBridge, 'loadStudy') + .mockRejectedValueOnce(new Error('backend down')) + .mockResolvedValue(undefined); + const { result } = renderHook(() => useGameSession()); + + await act(async () => { result.current.startSession(CONFIG); }); + // First load failed → stuck on the loading screen with an error. + expect(result.current.phase).toBe('loading'); + expect(result.current.loadError).toBe('backend down'); + + await act(async () => { result.current.retryStudy(); }); + expect(result.current.phase).toBe('playing'); + expect(result.current.loadError).toBeNull(); + expect(loadSpy).toHaveBeenCalledTimes(2); + }); + + it('finishEarly ends the session with the already-committed results (no data loss)', async () => { + vi.spyOn(gameBridge, 'loadStudy') + .mockResolvedValueOnce(undefined) // study 1 loads + .mockRejectedValue(new Error('backend down')); // study 2 fails + const { result } = renderHook(() => useGameSession()); + + await act(async () => { result.current.startSession(CONFIG); }); + await playAndAdvance(result, 0.8); // commit study 1, then study 2's load fails + expect(result.current.phase).toBe('loading'); + expect(result.current.loadError).toBe('backend down'); + expect(result.current.results).toHaveLength(1); + + await act(async () => { result.current.finishEarly(); }); + expect(result.current.phase).toBe('results'); + const log = result.current.sessionLog; + expect(log).not.toBeNull(); + expect(log!.studies).toHaveLength(1); + expect(log!.studies[0].studyId).toBe('s1'); + }); }); diff --git a/frontend/src/game/useGameSession.ts b/frontend/src/game/useGameSession.ts index dcc50e5d..68d2fbcb 100644 --- a/frontend/src/game/useGameSession.ts +++ b/frontend/src/game/useGameSession.ts @@ -30,6 +30,14 @@ export interface GameSessionState { advance: () => void; /** Abandon the session and return to the config screen. */ quit: () => void; + /** Retry loading the current study after a `loadError` (QW24). */ + retryStudy: () => void; + /** + * End the session NOW with whatever studies are already committed and jump + * to the results screen — so a mid-session backend failure doesn't destroy + * the completed results (QW24). + */ + finishEarly: () => void; } /** Best (lowest) resulting loading among the player's chosen actions. */ @@ -197,6 +205,28 @@ export function useGameSession(): GameSessionState { gameBridge.reset(); }, []); + // Retry the current study after a load failure — reuses loadStudyAt's own + // try/catch, so a transient failure clears loadError and reaches 'playing'. + const retryStudy = useCallback(() => { + void loadStudyAt(indexRef.current); + }, [loadStudyAt]); + + // Finish the session with whatever is already committed. Mirrors the + // session-complete branch of loadStudyAt but does NOT discard results — so a + // study that can't load no longer nukes the completed ones. + const finishEarly = useCallback(() => { + const cfg = configRef.current; + if (!cfg) return; + const log = buildSessionLog( + cfg, + resultsRef.current, + sessionStartRef.current, + new Date().toISOString(), + ); + setSessionLog(log); + setPhase('results'); + }, []); + return { phase, config, @@ -209,5 +239,7 @@ export function useGameSession(): GameSessionState { startSession, advance, quit, + retryStudy, + finishEarly, }; } diff --git a/recommender-pin.txt b/recommender-pin.txt new file mode 100644 index 00000000..3bb166aa --- /dev/null +++ b/recommender-pin.txt @@ -0,0 +1,9 @@ +# Single source of truth for the expert_op4grid_recommender version (QW8). +# PR / deploy installs pin to THIS exact version so an upstream release can't +# silently turn a green PR red or ship a regressed image on a zero-change +# rebuild. A weekly canary job (.github/workflows/canary.yml) floats to the +# latest release and opens an issue on failure, so upgrades are a deliberate +# bump of this line — not an implicit every-build resolve. +# +# Bump procedure: update this line, run the backend suite, review, commit. +expert_op4grid_recommender==0.3.0.post1 diff --git a/requirements_py310.txt b/requirements_py310.txt index af88e36b..c0c6c8bd 100644 --- a/requirements_py310.txt +++ b/requirements_py310.txt @@ -15,7 +15,7 @@ deepdiff==8.6.1 docutils==0.21.2 exceptiongroup==1.3.1 executing==2.2.1 -expert_op4grid_recommender==0.1.1 +expert_op4grid_recommender==0.3.0.post1 ExpertOp4Grid==0.3.2.post1 fastapi==0.129.0 fonttools==4.61.0 diff --git a/scripts/extract_network_zip.py b/scripts/extract_network_zip.py new file mode 100644 index 00000000..fd35ea38 --- /dev/null +++ b/scripts/extract_network_zip.py @@ -0,0 +1,52 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Validate + extract a Git-LFS-tracked network zip at Docker build time (QW25). + +The large grids ship as Git-LFS ``.zip`` files (they exceed HuggingFace's git +file limit). The Dockerfile previously extracted them with a bare one-liner that +had two silent failure modes: an un-smudged LFS *pointer* file (when ``git lfs +pull`` wasn't run) and a corrupt archive both produced a broken image with no +error. This script FAILS LOUDLY on either instead of shipping a broken grid. + + python scripts/extract_network_zip.py [...] +""" +from __future__ import annotations + +import sys +import zipfile +from pathlib import Path + +# Git-LFS pointer files begin with this line when the object wasn't smudged. +_LFS_POINTER_PREFIX = b"version https://git-lfs" + + +def validate_and_extract(zip_path: Path) -> bool: + """Validate and extract one zip. Returns False (skip) if absent; raises + SystemExit with a loud message on an LFS pointer or a corrupt archive.""" + if not zip_path.exists(): + print(f"{zip_path} absent — skipping") + return False + head = zip_path.read_bytes()[:64] + if head.startswith(_LFS_POINTER_PREFIX): + raise SystemExit( + f"FATAL: {zip_path} is an un-smudged Git-LFS pointer, not the real " + f"archive. Run `git lfs pull` before building the image." + ) + if not zipfile.is_zipfile(zip_path): + raise SystemExit(f"FATAL: {zip_path} is not a valid zip archive.") + zipfile.ZipFile(zip_path).extractall(zip_path.parent) + print(f"extracted {zip_path} -> {zip_path.parent}") + return True + + +def main(argv: list[str]) -> int: + for arg in argv[1:]: + validate_and_extract(Path(arg)) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/scripts/test_extract_network_zip.py b/scripts/test_extract_network_zip.py new file mode 100644 index 00000000..f6ac2386 --- /dev/null +++ b/scripts/test_extract_network_zip.py @@ -0,0 +1,46 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Tests for the Docker-build LFS-zip validator (QW25). Hermetic (stdlib only).""" +import importlib.util +import zipfile +from pathlib import Path + +import pytest + +_SPEC = importlib.util.spec_from_file_location( + "extract_network_zip", Path(__file__).resolve().parent / "extract_network_zip.py" +) +mod = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(mod) + + +def test_extracts_a_real_zip(tmp_path): + z = tmp_path / "network.xiidm.zip" + with zipfile.ZipFile(z, "w") as zf: + zf.writestr("network.xiidm", "") + assert mod.validate_and_extract(z) is True + assert (tmp_path / "network.xiidm").read_text() == "" + + +def test_absent_zip_is_skipped(tmp_path): + assert mod.validate_and_extract(tmp_path / "missing.zip") is False + + +def test_lfs_pointer_fails_loudly(tmp_path): + z = tmp_path / "network.xiidm.zip" + z.write_bytes( + b"version https://git-lfs.github.com/spec/v1\n" + b"oid sha256:deadbeef\nsize 12345\n" + ) + with pytest.raises(SystemExit, match="Git-LFS pointer"): + mod.validate_and_extract(z) + + +def test_corrupt_archive_fails_loudly(tmp_path): + z = tmp_path / "network.xiidm.zip" + z.write_bytes(b"not a zip at all") + with pytest.raises(SystemExit, match="not a valid zip"): + mod.validate_and_extract(z) diff --git a/scripts/test_recommender_pin_consistency.py b/scripts/test_recommender_pin_consistency.py new file mode 100644 index 00000000..d16a9d56 --- /dev/null +++ b/scripts/test_recommender_pin_consistency.py @@ -0,0 +1,55 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Consistency guard for the recommender version pin (QW8). + +recommender-pin.txt is the single source of truth for the pinned +expert_op4grid_recommender version. This hermetic (stdlib-only) test asserts +the PR/deploy install sites all consume it and that requirements_py310.txt +agrees, so the pin can't drift out of sync silently. +""" +import re +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +PIN_FILE = REPO / "recommender-pin.txt" + + +def _pinned_version() -> str: + for line in PIN_FILE.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if line and not line.startswith("#"): + m = re.fullmatch(r"expert_op4grid_recommender==(.+)", line) + assert m, f"unexpected pin line: {line!r}" + return m.group(1) + raise AssertionError("recommender-pin.txt has no pin line") + + +def test_pin_file_has_exact_version(): + v = _pinned_version() + assert re.fullmatch(r"\d+\.\d+\.\d+(?:\.post\d+)?", v), f"pin must be exact, got {v!r}" + + +def test_ci_and_docker_install_from_the_pin_file(): + test_yml = (REPO / ".github" / "workflows" / "test.yml").read_text(encoding="utf-8") + dockerfile = (REPO / "Dockerfile").read_text(encoding="utf-8") + # Both must install via the pin file, not a floating ">=" spec. + assert test_yml.count("-r recommender-pin.txt") >= 2, "test.yml must install the pin" + assert "recommender-pin.txt" in dockerfile, "Dockerfile must install the pin" + assert 'expert_op4grid_recommender>=' not in test_yml, "test.yml still has a floating spec" + assert 'expert_op4grid_recommender>=' not in dockerfile, "Dockerfile still has a floating spec" + + +def test_requirements_py310_matches_the_pin(): + v = _pinned_version() + reqs = (REPO / "requirements_py310.txt").read_text(encoding="utf-8") + assert f"expert_op4grid_recommender=={v}" in reqs, ( + "requirements_py310.txt must match recommender-pin.txt" + ) + + +def test_circleci_config_is_gone(): + # QW23 — collapsed to a single CI system (GitHub Actions). + assert not (REPO / ".circleci" / "config.yml").exists() From 8ba8291941a5b273ffcd74124fe675238858ca06 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 14:14:36 +0000 Subject: [PATCH 21/24] feat(game-mode): session-log replay to derive trusted scores (FU-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `--replay ` mode to scripts/game_mode/e2e_game_session.py that re-derives TRUSTED per-study finalMaxRho/solved from a session log's recorded actionsChosen by re-driving the real backend (config → step1 → step2), falling back to /api/simulate-manual-action for a manual pick that is no longer in the prioritized set. It writes a reference.json in the apply_reference() shape and flags studies whose replayed numbers diverge from the self-reported ones beyond --tolerance (tamper/drift detection; non-zero exit on divergence). - Extract configure_backend() + analyze_contingency() shared by the greedy play path and the replay path so trusted numbers come from one pipeline. - Hermetic coverage in scripts/game_mode/test_replay.py drives the replay machinery against a fake backend client (no FastAPI/pypowsybl/grid) — runs in the existing test-data-pipeline CI job. Real-grid reference generation stays an on-demand e2e-lane activity. - Docs: game-mode-codabench.md gains a --replay section; followups.md FU-2 marked landed (tooling + hermetic coverage). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- docs/architecture/followups.md | 115 ++++++++++++- docs/features/game-mode-codabench.md | 33 +++- scripts/game_mode/e2e_game_session.py | 238 ++++++++++++++++++++++---- scripts/game_mode/test_replay.py | 218 +++++++++++++++++++++++ 4 files changed, 569 insertions(+), 35 deletions(-) create mode 100644 scripts/game_mode/test_replay.py diff --git a/docs/architecture/followups.md b/docs/architecture/followups.md index 68e5d476..237ef6bc 100644 --- a/docs/architecture/followups.md +++ b/docs/architecture/followups.md @@ -75,7 +75,24 @@ is purely internal effect ordering, hence the extra behavioural coverage. ## FU-2 — Physically replay the exported Game Mode session log -**Status:** open · **Area:** game-mode / benchmark / trust · **Opened:** 2026-07-09 +**Status:** landed (tooling + hermetic coverage; real-grid reference runs in the +e2e lane) · **Area:** game-mode / benchmark / trust · **Opened:** 2026-07-09 · +**Resolved:** 2026-07-09 + +> **Resolution.** `scripts/game_mode/e2e_game_session.py` now has a +> `--replay ` mode: for each recorded study it re-drives the real +> backend (config → step1 → step2), re-derives the trusted `finalMaxRho` / +> `solved` from the log's recorded `actionsChosen` (falling back to +> `/api/simulate-manual-action` for an action no longer in the prioritized set), +> writes a trusted `reference.json` in the `apply_reference()` shape, and flags +> any study whose replayed numbers diverge from the self-reported ones beyond a +> `--tolerance` (tamper / drift detection; non-zero exit on divergence). The +> replay *machinery* is guarded hermetically by `scripts/game_mode/test_replay.py` +> (fake backend client — runs in CI from a fresh clone). What still requires the +> real FR/EUR grid bundle is *generating* the trusted reference for the public +> ranking, which stays an on-demand e2e-lane activity by design (same dependency +> the e2e harness already carries). The original write-up is kept below for +> context. ### Context @@ -118,3 +135,99 @@ partial implementation couldn't be guarded by a test and would rot. - `scripts/game_mode/e2e_game_session.py` (`play_study` — the replay machinery) - `scripts/game_mode/scoring_program/score.py` (`apply_reference`) - `frontend/src/game/types.ts` (`GameSessionLog` / `ChosenActionRecord`) + +--- + +## FU-3 — Memoize the per-study N-state flow / asset snapshot (QW12) + +**Status:** open · **Area:** backend / perf · **Opened:** 2026-07-09 + +### Context + +`DiagramMixin.get_contingency_diagram_patch` (≈ line 396–402) and +`_attach_flow_deltas_vs_base` (≈ line 859–863) both re-derive the **N-state** +base flow / asset snapshot the same way: pin the base network on +`self._get_n_variant()`, then call `get_network_flows()` + `get_asset_flows()` +and restore the variant. The N-state variant is a clean, immutable baseline for +the duration of a study (remedial actions run on separate variants), so this +snapshot is invariant per study and is recomputed on every contingency-patch and +action-variant delta call. + +### What's deferred (and why) + +Caching the snapshot on the service (`self._n_state_flows` / `self._n_state_assets`, +cleared in `reset()` alongside `_n_state_currents`) is the natural fix, but +`recommender_service.py` is at **exactly its 1150-line module ceiling** — adding +the two init + two reset lines cleanly would breach the code-quality gate, and +squeezing them in with semicolon-joined one-liners degrades the hub file the gate +protects. It is `do_with_care`: a missed invalidation would silently corrupt flow +deltas across studies (the class of bug `docs/features/state-reset-and-confirmation-dialogs.md` +exists to prevent), so it should not be rushed under ceiling pressure. The load +flow itself is already cached per variant, so the win is bounded to the repeated +DataFrame extraction (~100–400 ms/call on the 5 k-VL grid) — real but modest. + +### Suggested approach + +1. First reclaim ceiling headroom in `recommender_service.py` (extract a small + cohesive block, e.g. the config-globals push, into a helper) so the two cache + fields fit within 1150 without hacks. +2. Add `_n_state_flows` / `_n_state_assets` to `__init__` **and** `reset()` + (same group as `_n_state_currents`), plus the reset-completeness sweep in + `test_reset_completeness.py` will then require them automatically. +3. Add a `_get_n_state_flow_snapshot()` helper on `DiagramMixin` that lazily + computes + memoizes both, and route both call sites through it. +4. Document the new caches in `docs/features/state-reset-and-confirmation-dialogs.md`. + +### References + +- `expert_backend/services/diagram_mixin.py` (`get_contingency_diagram_patch`, + `_attach_flow_deltas_vs_base`, `_get_network_flows` / `_get_asset_flows`) +- `expert_backend/services/recommender_service.py` (`__init__` / `reset`, + `_n_state_currents` — the QW17 sibling cache to group with) +- `expert_backend/tests/test_reset_completeness.py` (the sweep that will guard it) + +--- + +## FU-4 — Emit a patch payload from `/api/simulate-and-variant-diagram` (QW13) + +**Status:** open · **Area:** backend + frontend / perf · **Opened:** 2026-07-09 + +### Context + +`/api/simulate-and-variant-diagram` streams `{type:"metrics"}` then +`{type:"diagram"}` with a **full** action-variant NAD SVG. The DOM-recycling fast +path (PR #108) already lets the frontend clone the mounted N-state SVG and patch +per-branch deltas for `/api/action-variant-diagram` via +`/api/action-variant-diagram-patch`, avoiding a multi-MB re-download. The combined +simulate-and-variant stream does not yet participate in that fast path. + +### What's deferred (and why) + +Making the stream emit an optional `{type:"patch"}` event (when the target SVG is +patchable against the mounted N-state clone) touches the backend endpoint **and +all three frontend stream consumers** that read this NDJSON, plus the +`svgPatch.ts` apply path and its fallback. It is `do_with_care`: a wrong +patchability decision produces a *visually* wrong diagram (stale branch colours / +missing VL subtree) that neither `tsc` nor the current tests catch — the same +class of silent regression FU-1 flags. It needs behavioural coverage of the +patch-vs-full branch before it ships, so it is not a drop-in quick win. + +### Suggested approach + +1. Reuse `services/diagram/action_patch.py` (the existing + `/api/action-variant-diagram-patch` pipeline) to compute a patch payload inside + the combined endpoint; emit `{type:"patch", ...}` **instead of** + `{type:"diagram"}` only when the snapshot is patchable, else fall back to the + full diagram event unchanged. +2. Teach the three stream consumers (search `simulate-and-variant` / + `parseNdjsonStream` usages) to route a `patch` event through + `applyPatchToClone`, with the full-diagram event as the untouched fallback. +3. Add a Vitest case asserting the patch branch reuses the mounted N-state clone + and the fallback still renders on an unpatchable payload. + +### References + +- `expert_backend/main.py` (`/api/simulate-and-variant-diagram`) +- `expert_backend/services/diagram/action_patch.py` +- `frontend/src/utils/svgPatch.ts` (`applyPatchToClone`), + `frontend/src/utils/ndjsonStream.ts` (the shared reader) diff --git a/docs/features/game-mode-codabench.md b/docs/features/game-mode-codabench.md index 65b1ba72..c59fd203 100644 --- a/docs/features/game-mode-codabench.md +++ b/docs/features/game-mode-codabench.md @@ -70,8 +70,8 @@ so a change to either scorer that breaks numerical parity fails a test in CI (the `test-data-pipeline` job runs the Python side; `test-frontend` runs the TS side). `apply_reference()` in `score.py` overlays trusted per-study numbers onto the self-reported log when a `reference.json` is supplied (a no-op -otherwise) — the physical session-log replay that would *produce* that trusted -reference is tracked as FU-2 in `docs/architecture/followups.md`. +otherwise); the physical session-log replay that *produces* that trusted +reference is the `--replay` mode below (FU-2 in `docs/architecture/followups.md`). ## Local end-to-end check @@ -89,6 +89,31 @@ This requires pypowsybl + expert_op4grid_recommender in the environment (the same deps the backend needs). The preset contingencies in `presets.ts` are all verified `can_proceed=True` by this script, so the game stays winnable. +### Replaying a session log (`--replay`, FU-2) + +An exported `GameSessionLog` carries *self-reported* `finalMaxRho` / `solved`. +To rank on **trusted** numbers, `--replay` re-derives them from the log's +recorded `actionsChosen` by re-driving the real backend: + +```bash +python3 scripts/game_mode/e2e_game_session.py \ + --replay path/to/game_session.json --grid fr \ + --reference-out test-results/replay_reference.json --tolerance 0.05 +# → writes a trusted reference.json (apply_reference() shape) and prints, per +# study, whether the replayed numbers diverge from the self-reported ones. +# Exits non-zero when any study diverges (tamper / drift signal). +``` + +For each study it runs `config → step1 → step2`, looks the recorded action ids +up in the freshly recomputed prioritized set (falling back to +`/api/simulate-manual-action` for a manual pick that is no longer prioritized), +takes the best (lowest) resulting `max_rho` as the trusted `finalMaxRho`, and +flags any action the backend can no longer reproduce. `--grid` selects the grid +the log was played on (all studies in a session share one difficulty tier). The +replay *machinery* is guarded hermetically by +`scripts/game_mode/test_replay.py` (a fake backend client — runs in CI); only +*generating a real reference* needs the FR/EUR grid bundle. + ## Tests - Frontend: `frontend/src/game/scoring.test.ts` (Vitest) — scoring + log/CSV @@ -98,3 +123,7 @@ verified `can_proceed=True` by this script, so the game stays winnable. `test-data-pipeline` job. Regenerate the fixture after a deliberate scorer change with `python scripts/game_mode/gen_scoring_golden.py` (then update `scoring.ts` to match). +- Replay (in-repo): `scripts/game_mode/test_replay.py` (pytest) — the FU-2 + session-log replay machinery against a fake backend client (action lookup, + manual-simulation fallback, best-of aggregation, divergence detection, + `reference.json` shape). Hermetic; runs in the same `test-data-pipeline` job. diff --git a/scripts/game_mode/e2e_game_session.py b/scripts/game_mode/e2e_game_session.py index e1d67ec7..8557c2e9 100644 --- a/scripts/game_mode/e2e_game_session.py +++ b/scripts/game_mode/e2e_game_session.py @@ -83,10 +83,9 @@ def parse_ndjson(text): yield json.loads(line) -def play_study(client, study, network, actions, layout, time_limit, max_actions): - """Drive config→step1→step2 for one study and return a GameStudyResult.""" - t0 = time.time() - cfg = { +def configure_backend(client, network, actions, layout): + """POST /api/config to load the network + action catalogue for a study.""" + r = client.post("/api/config", json={ "network_path": network, "action_file_path": actions, "layout_path": layout, @@ -94,58 +93,75 @@ def play_study(client, study, network, actions, layout, time_limit, max_actions) "pypowsybl_fast_mode": True, "model": "expert", "compute_overflow_graph": True, - } - r = client.post("/api/config", json=cfg) + }) r.raise_for_status() - contingency = study["contingencyElementId"] + +def analyze_contingency(client, contingency): + """Run step1 (+step2 when it can proceed) for one contingency. + + Returns ``(overloads, can_proceed, actions_dict, baseline_max_rho)`` — the + shared backend-driven analysis both the greedy play path and the log replay + consume, so trusted numbers come from the exact same pipeline. + """ r = client.post("/api/run-analysis-step1", json={"disconnected_elements": [contingency]}) r.raise_for_status() step1 = r.json() overloads = step1.get("lines_overloaded", []) - print(f" step1: {len(overloads)} overload(s), can_proceed={step1.get('can_proceed')}") + can_proceed = bool(step1.get("can_proceed")) - chosen = [] + actions_dict = {} baseline_max_rho = None - final_max_rho = None - if step1.get("can_proceed") and overloads: + if can_proceed and overloads: r = client.post("/api/run-analysis-step2", json={ "selected_overloads": overloads, "all_overloads": overloads, }) r.raise_for_status() - actions_dict = {} for event in parse_ndjson(r.text): if event.get("type") == "result": actions_dict = event.get("actions", {}) elif event.get("type") == "error": print(f" step2 error: {event.get('message')}") - print(f" step2: {len(actions_dict)} prioritized action(s)") - # Baseline worst loading: max rho_before across any action. for data in actions_dict.values(): rb = data.get("rho_before") or [] if rb: m = max(rb) baseline_max_rho = m if baseline_max_rho is None else max(baseline_max_rho, m) + return overloads, can_proceed, actions_dict, baseline_max_rho - # Greedy "player": pick up to max_actions actions with the lowest - # resulting max_rho (best remediation first). - ranked = sorted( - ((aid, d) for aid, d in actions_dict.items() if d.get("max_rho") is not None), - key=lambda kv: kv[1]["max_rho"], - ) - for aid, d in ranked[:max_actions]: - after = d.get("lines_overloaded_after") or [] - mr = d.get("max_rho") - solved = mr is not None and mr < 1.0 and len(after) == 0 - chosen.append({ - "actionId": aid, - "description": d.get("description_unitaire"), - "maxRho": mr, - "linesOverloadedAfter": after, - "solved": solved, - }) - final_max_rho = mr if final_max_rho is None else min(final_max_rho, mr) + +def play_study(client, study, network, actions, layout, time_limit, max_actions): + """Drive config→step1→step2 for one study and return a GameStudyResult.""" + t0 = time.time() + configure_backend(client, network, actions, layout) + + contingency = study["contingencyElementId"] + overloads, can_proceed, actions_dict, baseline_max_rho = analyze_contingency(client, contingency) + print(f" step1: {len(overloads)} overload(s), can_proceed={can_proceed}") + if can_proceed and overloads: + print(f" step2: {len(actions_dict)} prioritized action(s)") + + chosen = [] + final_max_rho = None + # Greedy "player": pick up to max_actions actions with the lowest + # resulting max_rho (best remediation first). + ranked = sorted( + ((aid, d) for aid, d in actions_dict.items() if d.get("max_rho") is not None), + key=lambda kv: kv[1]["max_rho"], + ) + for aid, d in ranked[:max_actions]: + after = d.get("lines_overloaded_after") or [] + mr = d.get("max_rho") + solved = mr is not None and mr < 1.0 and len(after) == 0 + chosen.append({ + "actionId": aid, + "description": d.get("description_unitaire"), + "maxRho": mr, + "linesOverloadedAfter": after, + "solved": solved, + }) + final_max_rho = mr if final_max_rho is None else min(final_max_rho, mr) duration_ms = int((time.time() - t0) * 1000) solved = final_max_rho is not None and final_max_rho < 1.0 @@ -169,6 +185,153 @@ def play_study(client, study, network, actions, layout, time_limit, max_actions) } +# ---------------------------------------------------------------------------- +# Session-log replay (FU-2): re-derive TRUSTED per-study numbers from a session +# log's recorded actions by re-driving the real backend, so the Codabench +# scorer ranks replayed numbers instead of self-reported ones. +# ---------------------------------------------------------------------------- + +def replay_action(client, contingency, action_id, actions_dict, overloads): + """Trusted outcome of one recorded action against ``contingency``. + + Prefer the freshly recomputed step-2 prioritized entry (the same source the + UI recorded from); fall back to a direct ``/api/simulate-manual-action`` for + an action that is no longer in the prioritized set (manual pick / reorder). + Returns ``(max_rho, lines_overloaded_after, found)``. + """ + d = actions_dict.get(action_id) + if d is None: + r = client.post("/api/simulate-manual-action", json={ + "action_id": action_id, + "disconnected_elements": [contingency], + "lines_overloaded": overloads, + }) + if getattr(r, "status_code", 200) == 200: + d = r.json() + if d is None: + return None, [], False + return d.get("max_rho"), (d.get("lines_overloaded_after") or []), True + + +def replay_study(client, recorded, network, actions, layout, tolerance): + """Re-derive trusted numbers for one recorded ``GameStudyResult``. + + Returns ``(reference_entry, divergence)``. ``reference_entry`` is in the + :func:`score.apply_reference` shape (``studyId`` / ``baselineMaxRho`` / + ``finalMaxRho`` / ``solved``). ``divergence`` reports, per numeric field, + whether the replayed value differs from the self-reported one beyond + ``tolerance`` (tamper / drift detection) and lists any action id the backend + could no longer reproduce. + """ + configure_backend(client, network, actions, layout) + contingency = recorded["contingencyElementId"] + overloads, can_proceed, actions_dict, baseline_max_rho = analyze_contingency(client, contingency) + + trusted_final = None + missing = [] + for rec in recorded.get("actionsChosen", []): + aid = rec.get("actionId") + mr, _after, found = replay_action(client, contingency, aid, actions_dict, overloads) + if not found: + missing.append(aid) + continue + if mr is not None: + trusted_final = mr if trusted_final is None else min(trusted_final, mr) + + trusted_solved = trusted_final is not None and trusted_final < 1.0 + reference_entry = { + "studyId": recorded["studyId"], + "baselineMaxRho": baseline_max_rho, + "finalMaxRho": trusted_final, + "solved": trusted_solved, + } + divergence = diff_reference(recorded, reference_entry, missing, tolerance) + return reference_entry, divergence + + +def diff_reference(recorded, reference_entry, missing, tolerance): + """Compare self-reported vs. replayed numbers; return a per-study report.""" + fields = {} + ok = True + for key in ("baselineMaxRho", "finalMaxRho"): + reported = recorded.get(key) + trusted = reference_entry.get(key) + if reported is None or trusted is None: + match = reported is None and trusted is None + delta = None + else: + delta = abs(float(reported) - float(trusted)) + match = delta <= tolerance + ok = ok and match + fields[key] = {"reported": reported, "replayed": trusted, "delta": delta, "match": match} + solved_match = bool(recorded.get("solved")) == bool(reference_entry.get("solved")) + ok = ok and solved_match and not missing + fields["solved"] = { + "reported": bool(recorded.get("solved")), + "replayed": bool(reference_entry.get("solved")), + "match": solved_match, + } + return {"studyId": recorded["studyId"], "ok": ok, "fields": fields, "missingActions": missing} + + +def replay_session(client, session, network, actions, layout, tolerance): + """Replay every study in a session log; return (reference, divergences).""" + reference_studies = [] + divergences = [] + for i, recorded in enumerate(session.get("studies", [])): + print(f"[replay {i + 1}/{len(session['studies'])}] {recorded.get('label', recorded['studyId'])}") + ref, div = replay_study(client, recorded, network, actions, layout, tolerance) + reference_studies.append(ref) + divergences.append(div) + status = "OK" if div["ok"] else "DIVERGES" + print(f" {status}: finalMaxRho reported={div['fields']['finalMaxRho']['reported']} " + f"replayed={div['fields']['finalMaxRho']['replayed']}" + + (f" MISSING {div['missingActions']}" if div["missingActions"] else "")) + return {"studies": reference_studies}, divergences + + +def grid_paths(grid): + """Resolve the (network, actions, layout) triple for a difficulty grid.""" + if grid == "fr": + return FR_NETWORK, FR_ACTIONS, FR_LAYOUT + return ( + "data/bare_env_small_grid_test/grid.xiidm", + "data/action_space/reduced_model_actions_test.json", + "", + ) + + +def run_replay(args): + """`--replay` entry point: re-derive trusted numbers from a session log.""" + with open(args.replay, encoding="utf-8") as fh: + session = json.load(fh) + network, actions, layout = grid_paths(args.grid) + + sys.path.insert(0, REPO) + from fastapi.testclient import TestClient + from expert_backend.main import app + + with TestClient(app) as client: + reference, divergences = replay_session( + client, session, network, actions, layout, args.tolerance, + ) + + os.makedirs(os.path.dirname(args.reference_out), exist_ok=True) + with open(args.reference_out, "w", encoding="utf-8") as fh: + json.dump(reference, fh, indent=2) + print(f"\nWrote trusted reference -> {args.reference_out}") + + diverged = [d for d in divergences if not d["ok"]] + print("=== Replay verification ===") + print(json.dumps({ + "studies": len(divergences), + "diverged": len(diverged), + "divergedStudyIds": [d["studyId"] for d in diverged], + }, indent=2)) + # Non-zero exit signals tamper / drift so a CI lane can gate on it. + return 1 if diverged else 0 + + def main(): ap = argparse.ArgumentParser() ap.add_argument("--grid", choices=["fr", "small"], default="fr") @@ -177,9 +340,20 @@ def main(): ap.add_argument("--time-limit", type=int, default=300) ap.add_argument("--out", default=os.path.join(REPO, "test-results", "e2e_game_session.json")) ap.add_argument("--scorer", default=DEFAULT_SCORER) + # FU-2 replay mode: re-derive trusted numbers from an exported session log. + ap.add_argument("--replay", default=None, + help="Replay an exported session log to derive trusted reference numbers.") + ap.add_argument("--reference-out", + default=os.path.join(REPO, "test-results", "replay_reference.json"), + help="Where to write the trusted reference.json (replay mode).") + ap.add_argument("--tolerance", type=float, default=0.05, + help="Max |reported − replayed| before a study is flagged as diverging.") args = ap.parse_args() os.chdir(REPO) + if args.replay: + sys.exit(run_replay(args)) + sys.path.insert(0, REPO) from fastapi.testclient import TestClient from expert_backend.main import app diff --git a/scripts/game_mode/test_replay.py b/scripts/game_mode/test_replay.py new file mode 100644 index 00000000..cc5162e0 --- /dev/null +++ b/scripts/game_mode/test_replay.py @@ -0,0 +1,218 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +# This file is part of Co-Study4Grid a Power Grid Study tool Assistant Interface. +"""Hermetic guard for the Game Mode session-log replay (FU-2). + +Exercises the replay MACHINERY in scripts/game_mode/e2e_game_session.py — +action lookup, fallback to /api/simulate-manual-action, min-rho aggregation, +divergence detection, and the reference.json shape — against a FAKE backend +client returning canned responses. No FastAPI, pypowsybl, or grid data, so it +runs in CI from a fresh clone. The real-backend replay stays in the e2e lane +(`e2e_game_session.py --replay `). +""" +import importlib.util +import json +import os + +import pytest + +HERE = os.path.dirname(os.path.abspath(__file__)) + + +def _load_e2e(): + spec = importlib.util.spec_from_file_location( + "e2e_game_session", os.path.join(HERE, "e2e_game_session.py") + ) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def _load_scorer(): + spec = importlib.util.spec_from_file_location( + "score", os.path.join(HERE, "scoring_program", "score.py") + ) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +@pytest.fixture(scope="module") +def e2e(): + return _load_e2e() + + +@pytest.fixture(scope="module") +def score(): + return _load_scorer() + + +class FakeResponse: + def __init__(self, *, json_body=None, text="", status_code=200): + self._json = json_body + self.text = text + self.status_code = status_code + + def raise_for_status(self): + if self.status_code >= 400: + raise RuntimeError(f"HTTP {self.status_code}") + + def json(self): + return self._json + + +class FakeClient: + """Canned backend: one N-1 with two prioritized actions. + + - ``a_solve`` → max_rho 0.85, no residual overloads (a full solve) + - ``a_partial``→ max_rho 1.10, one residual overload + - ``a_manual`` → NOT in the step-2 set; only reachable via the + /api/simulate-manual-action fallback (returns max_rho 0.95). + Any other simulate target 404s (missing action). + """ + + STEP2_ACTIONS = { + "a_solve": { + "description_unitaire": "Solve action", + "rho_before": [1.4, 1.2], "max_rho": 0.85, "lines_overloaded_after": [], + }, + "a_partial": { + "description_unitaire": "Partial action", + "rho_before": [1.4, 1.2], "max_rho": 1.10, "lines_overloaded_after": ["L9"], + }, + } + + def __init__(self): + self.calls = [] + + def post(self, url, json=None): # noqa: A002 - mirror requests' kwarg name + self.calls.append(url) + if url == "/api/config": + return FakeResponse(json_body={"ok": True}) + if url == "/api/run-analysis-step1": + return FakeResponse(json_body={ + "lines_overloaded": ["L9", "L12"], "can_proceed": True, + }) + if url == "/api/run-analysis-step2": + ndjson = "\n".join([ + json_dumps({"type": "pdf", "pdf_url": "x"}), + json_dumps({"type": "result", "actions": self.STEP2_ACTIONS}), + ]) + return FakeResponse(text=ndjson) + if url == "/api/simulate-manual-action": + aid = json.get("action_id") + if aid == "a_manual": + return FakeResponse(json_body={ + "max_rho": 0.95, "lines_overloaded_after": [], + }) + return FakeResponse(json_body={"detail": "unknown"}, status_code=404) + raise AssertionError(f"unexpected url {url}") + + +def json_dumps(obj): + return json.dumps(obj) + + +def _recorded(action_ids, *, final, baseline=1.4, solved=True): + return { + "studyId": "s1", "label": "Study 1", + "contingencyElementId": "ctg1", "contingencyLabel": "Ctg 1", + "actionsChosen": [{"actionId": a, "maxRho": None, "solved": False} for a in action_ids], + "numActions": len(action_ids), + "baselineMaxRho": baseline, "finalMaxRho": final, "solved": solved, + } + + +def test_replay_matches_faithful_report(e2e): + # Player reported the solve action's true number → replay agrees. + recorded = _recorded(["a_solve"], final=0.85, baseline=1.4, solved=True) + ref, div = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + assert ref == { + "studyId": "s1", "baselineMaxRho": 1.4, "finalMaxRho": 0.85, "solved": True, + } + assert div["ok"] is True + assert div["fields"]["finalMaxRho"]["match"] is True + assert div["missingActions"] == [] + + +def test_replay_takes_best_of_multiple_actions(e2e): + # Both actions chosen: trusted finalMaxRho is the MIN (best remediation). + recorded = _recorded(["a_partial", "a_solve"], final=0.85, solved=True) + ref, div = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + assert ref["finalMaxRho"] == 0.85 + assert ref["solved"] is True + assert div["ok"] is True + + +def test_replay_flags_inflated_self_report(e2e): + # Player claims 0.80 but the only chosen action truly yields 1.10 → diverges, + # and the trusted solve flag flips to False. + recorded = _recorded(["a_partial"], final=0.80, baseline=1.4, solved=True) + ref, div = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + assert ref["finalMaxRho"] == 1.10 + assert ref["solved"] is False + assert div["ok"] is False + assert div["fields"]["finalMaxRho"]["match"] is False + assert div["fields"]["solved"]["match"] is False + + +def test_replay_falls_back_to_manual_simulation(e2e): + # a_manual is not in the step-2 set → replay_action must reach for + # /api/simulate-manual-action and still derive a trusted number. + client = FakeClient() + recorded = _recorded(["a_manual"], final=0.95, solved=True) + ref, div = e2e.replay_study(client, recorded, "net", "act", "", 0.05) + assert ref["finalMaxRho"] == 0.95 + assert "/api/simulate-manual-action" in client.calls + assert div["ok"] is True + assert div["missingActions"] == [] + + +def test_replay_reports_unreproducible_action(e2e): + # An action the backend can neither prioritize nor simulate is flagged + # missing and forces a divergence even if the numbers happen to align. + recorded = _recorded(["ghost"], final=None, baseline=1.4, solved=False) + ref, div = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + assert ref["finalMaxRho"] is None + assert div["missingActions"] == ["ghost"] + assert div["ok"] is False + + +def test_replay_within_tolerance_is_ok(e2e): + # Reported 0.88 vs replayed 0.85 — inside the 0.05 tolerance band. + recorded = _recorded(["a_solve"], final=0.88, baseline=1.42, solved=True) + ref, div = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + assert div["fields"]["finalMaxRho"]["match"] is True + assert div["fields"]["baselineMaxRho"]["match"] is True + assert div["ok"] is True + + +def test_reference_entry_feeds_apply_reference(e2e, score): + # The reference.json the replay emits must be consumable by the scorer's + # apply_reference(), overriding the self-reported numbers end to end. + recorded = _recorded(["a_partial"], final=0.5, baseline=1.4, solved=True) + ref_entry, _ = e2e.replay_study(FakeClient(), recorded, "net", "act", "", 0.05) + session = {"studies": [dict(recorded, numActions=1, maxActions=3, + timeLimitSeconds=300, durationMs=30000)]} + score.apply_reference(session, {"studies": [ref_entry]}) + # Trusted numbers now scored: the inflated "solved" self-report is corrected. + assert session["studies"][0]["finalMaxRho"] == 1.10 + assert session["studies"][0]["solved"] is False + assert score.score_study(session["studies"][0])["remediationFraction"] < 1.0 + + +def test_replay_session_aggregates_divergences(e2e): + session = {"studies": [ + _recorded(["a_solve"], final=0.85, solved=True), + dict(_recorded(["a_partial"], final=0.80, solved=True), studyId="s2"), + ]} + reference, divergences = e2e.replay_session( + FakeClient(), session, "net", "act", "", 0.05, + ) + assert len(reference["studies"]) == 2 + assert [d["ok"] for d in divergences] == [True, False] + # reference is serialisable (it is written to disk as reference.json). + assert json.loads(json.dumps(reference))["studies"][0]["studyId"] == "s1" From 66a90724bddffdf99d48735bb40be2facf058e7b Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 14:23:20 +0000 Subject: [PATCH 22/24] feat(docs): gate the CLAUDE.md inventory against drift (D9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recurring "docs drift" review finding — CLAUDE.md trees referencing renamed-away files, and file.py:NNN line anchors that had rotted by hundreds of lines — is now a gate instead of a periodic manual audit. scripts/check_docs_tree.py scans the four inventory docs and fails on: A. a directory-qualified `path.ext` reference that resolves to no real file (exempting generated artifacts and paths described AS removed, via a ±1-line window so wrapped prose is handled); B. any `file.py:NNN` line-number anchor — the convention is a symbol anchor (name the function/class), which survives edits. - Converted all seven pre-existing line anchors to symbol anchors and fixed the two drifted path references (StatusToasts row annotated as superseded by NotificationHost; _service_integration already described as removed). - Wired as its own gate step in .github/workflows/code-quality.yml. - scripts/test_check_docs_tree.py: fixture-driven classification + a real-repo self-guard so drift fails the pytest suite too, not only the CI step. - Docs: code-quality-analysis.md §23; review D8/D9 status marked done. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- .github/workflows/code-quality.yml | 10 +- docs/architecture/2026-07-full-repo-review.md | 19 +- docs/architecture/code-quality-analysis.md | 33 +++ expert_backend/CLAUDE.md | 12 +- frontend/CLAUDE.md | 6 +- scripts/check_docs_tree.py | 197 ++++++++++++++++++ scripts/test_check_docs_tree.py | 114 ++++++++++ 7 files changed, 374 insertions(+), 17 deletions(-) create mode 100644 scripts/check_docs_tree.py create mode 100644 scripts/test_check_docs_tree.py diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 6673874f..c6ea6a81 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -31,8 +31,14 @@ jobs: - name: Run code-quality gate run: python scripts/check_code_quality.py - - name: Run reporter unit tests - run: pytest scripts/test_code_quality_report.py -q + - name: Run docs-tree gate (D9) + # Fails when a CLAUDE.md reference points at a file that no longer exists + # or reintroduces a rotting `file.py:NNN` line anchor. See + # docs/architecture/code-quality-analysis.md §23. + run: python scripts/check_docs_tree.py + + - name: Run reporter + docs-tree unit tests + run: pytest scripts/test_code_quality_report.py scripts/test_check_docs_tree.py -q - name: Run mypy (gate) # GATES the build: the shared-state base (services/_recommender_state.py) diff --git a/docs/architecture/2026-07-full-repo-review.md b/docs/architecture/2026-07-full-repo-review.md index c11407ae..4bd75ab6 100644 --- a/docs/architecture/2026-07-full-repo-review.md +++ b/docs/architecture/2026-07-full-repo-review.md @@ -560,8 +560,8 @@ Ordered by leverage; each unblocks or de-risks the ones after it. | D5 | One streaming + notification pipeline | ✅ Done | — | | D6 | SVG element-adoption pipeline | 🟢 Core done | VL-count auto-enable of bitmap pan/zoom (validation-gated on operator hardware) | | D7 | Deployment trust & reproducibility | 🟡 Mostly | pinned-Python-closure lockfile; Dockerfile dead-weight / `HEALTHCHECK` / config-merge | -| D8 | Reproducible data & benchmark supply chain | 🟢 Mostly | **FU-2** — physically replay the exported session log (trusted, not self-reported, ranking numbers) | -| D9 | Docs as a checked artifact | ⬜ Open | not started | +| D8 | Reproducible data & benchmark supply chain | ✅ Done | **FU-2** landed — `e2e_game_session.py --replay` re-derives trusted ranking numbers + hermetic `test_replay.py` | +| D9 | Docs as a checked artifact | ✅ Done | `scripts/check_docs_tree.py` gate + symbol anchors + `test_check_docs_tree.py`; CLAUDE.md tree brought green | Full accounting of every open item — deep-revision tails, follow-ups, and the remaining quick wins — is in **[§ Part V.5 — What's left](#part-v5--whats-left-2026-07-09)** below. @@ -832,11 +832,16 @@ in rough priority order: **Deep revisions still open or with a tail** -- **D9 — docs as a checked artifact** *(not started; highest-value remaining - structural item)*. `check_docs_tree.py` in the gate + symbol anchors + slimmed - CLAUDE.md trees. The review's own "docs drift" finding keeps recurring (its - line-number anchors are already stale — see Part VII), so this is the item that - stops the inventory layer from rotting again. +- **D9 — docs as a checked artifact** *(✅ landed 2026-07-09)*. + `scripts/check_docs_tree.py` is now a gate step in + `.github/workflows/code-quality.yml`: it fails when a `CLAUDE.md` reference + points at a file that no longer exists (with generated-artifact and + referenced-as-removed exemptions) or reintroduces a rotting `file.py:NNN` line + anchor. All seven pre-existing line anchors were converted to symbol anchors + and the two drifted path references fixed, so the tree is green. Unit coverage + + a real-repo self-guard live in `scripts/test_check_docs_tree.py`; full + write-up in [`code-quality-analysis.md`](code-quality-analysis.md) §23. This + closes the recurring inventory-layer drift finding. - **D2 tail** — the machine-check backbone + `{detail, code}` envelope shipped; still open: response models on the gzipped endpoints, generating `types.ts` from `openapi.snapshot.json`, and removing the blanket exception handler. See diff --git a/docs/architecture/code-quality-analysis.md b/docs/architecture/code-quality-analysis.md index 0a4fd058..d1af3ff5 100644 --- a/docs/architecture/code-quality-analysis.md +++ b/docs/architecture/code-quality-analysis.md @@ -1338,5 +1338,38 @@ Headline margins at hand-off (ceiling − current): Also surfaced in `CHANGELOG.md` `[Unreleased]` so it is bound to the next release. +--- + +## 23. Delta — 2026-07-09 (D9: docs as a checked artifact) + +The review's recurring "docs drift" finding — the `CLAUDE.md` inventory +trees reference files that were renamed away, and their `file.py:NNN` +line anchors rot on the next edit (all seven had drifted by hundreds of +lines) — is now a **gate**, not a periodic manual audit. + +`scripts/check_docs_tree.py` scans the four inventory docs (root + +`expert_backend/` + `expert_backend/tests/` + `frontend/`) and fails on: + +1. **Referenced-path existence** — every backtick-quoted, directory- + qualified `path/like/this.ext` must resolve to a real file under any + sensible base dir (the docs write paths relative to their own + subtree). Exemptions: generated/runtime artifacts + (`dist-standalone/standalone.html`, `Overflow_Graph/…`) and paths + referenced *as removed* (the line — or a ±1-line window, for wrapped + prose — carries "removed / former / renamed / superseded / …"). +2. **No stale line-number anchors** — `file.py:NNN` is forbidden; the + convention is a **symbol anchor** (name the function/class, which + survives edits). The seven pre-existing anchors were converted (e.g. + `network_service.py:352` → "the module-level `NetworkService()` + singleton in `network_service.py`"). + +Wired into `.github/workflows/code-quality.yml` as its own gate step, +with unit coverage in `scripts/test_check_docs_tree.py` (fixture-driven +classification + a self-guard asserting the real repo stays clean, so +drift fails the pytest suite too, not only the CI step). A `--warn-only` +flag supports a roll-in period; the tree was brought fully green before +the gate was made blocking. This closes the last "inventory layer rots" +finding from the 2026-07 review (D9). + diff --git a/expert_backend/CLAUDE.md b/expert_backend/CLAUDE.md index 099aee49..495d0041 100644 --- a/expert_backend/CLAUDE.md +++ b/expert_backend/CLAUDE.md @@ -161,11 +161,13 @@ working unchanged. ## Singletons & shared state -- `network_service` (`services/network_service.py:352`) — owns the +- `network_service` (the module-level `NetworkService()` singleton in + `services/network_service.py`) — owns the `pypowsybl.network.Network` returned by `pn.load()`. Read-only consumers (frontend `/api/branches`, `/api/voltage-levels`, …) go through it. -- `recommender_service` (`services/recommender_service.py:727`) — owns +- `recommender_service` (the module-level `RecommenderService()` singleton in + `services/recommender_service.py`) — owns analysis state. `_get_base_network()` MUTUALISES the same Network object loaded by `network_service` to avoid re-parsing the .xiidm twice (~3-5 s on the PyPSA-EUR France grid). See @@ -180,7 +182,7 @@ The shared Network is safe because: lock — see "Concurrency ownership" below. `pn.load()` is called WITHOUT `allow_variant_multi_thread_access=True` -on purpose — see the long comment at `network_service.py:30-43` for +on purpose — see the long `allow_variant_multi_thread_access` comment in `network_service.py` for why enabling that is unsafe for the FastAPI thread pool today. ## Concurrency ownership (D3, 2026-07) @@ -401,7 +403,7 @@ Do NOT route streaming endpoints through `_maybe_gzip_*`. The per-endpoint gzip helper is for non-streaming responses only — wrapping NDJSON in gzip buffers events until a flush, breaking the early-PDF guarantee. This was the root cause behind the global -`GZipMiddleware` rollback (`main.py:30-42`, +`GZipMiddleware` rollback (see the rollback note atop `main.py`, `docs/performance/history/per-endpoint-gzip.md`). ## Per-endpoint gzip @@ -415,7 +417,7 @@ Two helpers in `main.py`: client-side `JSON.parse` on the multi-MB SVG string. Both set `Vary: Accept-Encoding`. Threshold and compression level are -tunable at `main.py:43-44`. +tunable via the `_GZIP_MIN_BYTES` / `_GZIP_LEVEL` constants in `main.py`. ## NumPy → JSON sanitization diff --git a/frontend/CLAUDE.md b/frontend/CLAUDE.md index 4efd9b78..8fcf26b4 100644 --- a/frontend/CLAUDE.md +++ b/frontend/CLAUDE.md @@ -304,7 +304,7 @@ silently (the `redispatch_details` save-drop bug). ## State reset & confirmation dialogs -`resetAllState()` (`App.tsx:310-324`) clears every per-study piece of +`resetAllState()` (in `App.tsx`) clears every per-study piece of React state. It is called on Apply Settings AND on Load Study. The backend mirrors this with `recommender_service.reset()` — `docs/features/state-reset-and-confirmation-dialogs.md` is the contract for @@ -356,7 +356,7 @@ mirrored in `scripts/check_standalone_parity.py`'s `SPEC_DETAILS`. The frontend deals with multi-MB pypowsybl SVG payloads. Five performance levers are applied today: -- **`api.getNetworkDiagram` uses `format=text`** (`api.ts:69-92`): +- **`api.getNetworkDiagram` uses `format=text`** (`getNetworkDiagram` in `api.ts`): fetches a JSON-header + raw-SVG-body response so the browser doesn't `JSON.parse` a 25 MB string. Saves ~500 ms on large grids. - **Element-adoption ingestion** (`utils/svg/svgBoost.ts` @@ -720,7 +720,7 @@ what has been extracted and what remains deferred. |---|---|---| | Sticky contingency/N-1 summary strip | `components/SidebarSummary.tsx` | ~90 | | Sidebar layout shell (summary + contingency selector + children slot) | `components/AppSidebar.tsx` | ~160 | -| Error / info floating toasts | `components/StatusToasts.tsx` | ~25 | +| Error / info floating toasts | `components/StatusToasts.tsx` (later superseded by `NotificationHost.tsx`, D5) | ~25 | | N-1 diagram fetch effect (svgPatch fast-path + `/api/contingency-diagram` fallback + contingency-change confirm routing) | `hooks/useContingencyFetch.ts` | ~120 | | `applyHighlightsForTab` + driving effect + per-tab `detachedViewModes` state + `viewModeForTab` / `handleViewModeChangeForTab` | `hooks/useDiagramHighlights.ts` | ~155 | diff --git a/scripts/check_docs_tree.py b/scripts/check_docs_tree.py new file mode 100644 index 00000000..bc80ede3 --- /dev/null +++ b/scripts/check_docs_tree.py @@ -0,0 +1,197 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""D9 — docs as a checked artifact. + +The hand-maintained ``CLAUDE.md`` inventory docs (root + per-subtree) carry an +elaborate file tree and hundreds of ``path/like/this.py`` references. They drift: +files get renamed / removed and the prose keeps pointing at the old name, and +``file.py:352`` line-number anchors rot on the very next edit. This gate keeps +that inventory layer honest with two checks: + + A. **Referenced-path existence.** Every backtick-quoted, directory-qualified + path with a source/asset extension must resolve to a real file (under any + sensible base dir — the docs write paths relative to their own subtree), + UNLESS it is a known generated/runtime artifact or is referenced *as + removed* (its line says "removed / former / renamed / superseded / …"). + + B. **No stale line-number anchors.** ``foo.py:352`` anchors are forbidden; + the convention is a **symbol anchor** — name the function / class instead, + which survives edits. (The review's own anchors had already rotted by + hundreds of lines; this stops that class of drift recurring.) + +Usage:: + + python scripts/check_docs_tree.py # gate: non-zero on any finding + python scripts/check_docs_tree.py --warn-only # report only, always exit 0 + python scripts/check_docs_tree.py --json # machine-readable findings + +The module also exposes ``scan_docs(repo_root)`` returning structured findings so +``scripts/test_check_docs_tree.py`` can drive it hermetically on tmp fixtures. +""" +from __future__ import annotations + +import argparse +import json +import re +import sys +from dataclasses import dataclass, field +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# The inventory docs this gate guards. Add a doc here to bring it under the gate. +DOC_FILES = [ + "CLAUDE.md", + "expert_backend/CLAUDE.md", + "expert_backend/tests/CLAUDE.md", + "frontend/CLAUDE.md", +] + +# Paths in these docs are written relative to a contextual base (a frontend doc +# says ``components/Foo.tsx`` meaning ``frontend/src/components/Foo.tsx``). A +# reference is satisfied if it exists under ANY of these bases — we care that the +# file EXISTS somewhere sensible, not that its relative form is canonical. +BASE_DIRS = [ + ".", + "frontend", + "frontend/src", + "expert_backend", + "expert_backend/tests", + "scripts", + "scripts/game_mode", + "scripts/pypsa_eur", + "docs", + "data", + "deploy", +] + +# A directory-qualified path with a source/asset extension, in backticks. +PATH_RE = re.compile(r"`([A-Za-z0-9_./-]+/[A-Za-z0-9_./-]+\.(?:py|ts|tsx|css|json|html|md))`") + +# A rotting line-number anchor: ``file.py:352`` or ``file.tsx:310-324``. +ANCHOR_RE = re.compile(r"\b([A-Za-z0-9_./-]+\.(?:py|ts|tsx|css|json|html)):(\d+(?:-\d+)?)\b") + +# Generated / runtime artifacts that legitimately do not exist in a fresh clone. +# Matched as a suffix of the referenced path. +GENERATED_SUFFIXES = ( + "dist-standalone/standalone.html", + "frontend/dist/index.html", + "reports/code-quality.json", + "reports/code-quality.md", +) +# Generated / runtime path segments (matched anywhere in the reference). +GENERATED_SEGMENTS = ( + "Overflow_Graph/", + "/dist/", + "test-results/", + "node_modules/", +) + +# When a reference's own line carries one of these words it is being described +# AS gone (a removed module, a renamed file) — existence must NOT be asserted. +REMOVED_RE = re.compile( + r"\b(removed|former|formerly|deleted|decommission|decommissioned|renamed|" + r"superseded|supersedes|no longer|legacy|replaced by|replaces the|frozen)\b", + re.IGNORECASE, +) + + +@dataclass +class Findings: + missing_paths: list[tuple[str, int, str]] = field(default_factory=list) + line_anchors: list[tuple[str, int, str]] = field(default_factory=list) + + @property + def total(self) -> int: + return len(self.missing_paths) + len(self.line_anchors) + + def as_dict(self) -> dict: + return { + "missing_paths": [ + {"doc": d, "line": ln, "ref": r} for d, ln, r in self.missing_paths + ], + "line_anchors": [ + {"doc": d, "line": ln, "anchor": a} for d, ln, a in self.line_anchors + ], + "total": self.total, + } + + +def _is_generated(ref: str) -> bool: + if any(ref.endswith(sfx) for sfx in GENERATED_SUFFIXES): + return True + return any(seg in ref for seg in GENERATED_SEGMENTS) + + +def _resolves(ref: str, repo_root: Path) -> bool: + return any((repo_root / base / ref).exists() for base in BASE_DIRS) + + +def scan_docs(repo_root: Path, doc_files: list[str] | None = None) -> Findings: + """Scan the inventory docs and return structured findings. + + Pure and side-effect free (no printing) so tests can drive it on fixtures. + """ + findings = Findings() + for doc in doc_files if doc_files is not None else DOC_FILES: + doc_path = repo_root / doc + if not doc_path.exists(): + continue + lines = doc_path.read_text(encoding="utf-8").splitlines() + for lineno, line in enumerate(lines, 1): + # Prose wraps, so a "removed / renamed / …" qualifier can sit on the + # line before or after the reference itself — scan a ±1 window. + window = " ".join(lines[max(0, lineno - 2):lineno + 1]) + removed_context = bool(REMOVED_RE.search(window)) + for m in PATH_RE.finditer(line): + ref = m.group(1) + if removed_context or _is_generated(ref): + continue + if not _resolves(ref, repo_root): + findings.missing_paths.append((doc, lineno, ref)) + for m in ANCHOR_RE.finditer(line): + findings.line_anchors.append((doc, lineno, f"{m.group(1)}:{m.group(2)}")) + return findings + + +def _report(findings: Findings) -> None: + if findings.missing_paths: + print("Docs reference files that do not exist (rename/remove drift):") + for doc, ln, ref in findings.missing_paths: + print(f" - {doc}:{ln} → `{ref}`") + print(" Fix: update the path, or note it as removed/generated on the same line.") + if findings.line_anchors: + print("Docs use stale line-number anchors (they rot on every edit):") + for doc, ln, anchor in findings.line_anchors: + print(f" - {doc}:{ln} → `{anchor}`") + print(" Fix: replace `file.ext:NNN` with a SYMBOL anchor (name the " + "function/class), e.g. `network_service.py` (`load_network`).") + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description="D9 docs-as-checked-artifact gate.") + ap.add_argument("--warn-only", action="store_true", + help="Report findings but always exit 0 (roll-in mode).") + ap.add_argument("--json", action="store_true", help="Emit findings as JSON.") + args = ap.parse_args(argv) + + findings = scan_docs(REPO_ROOT) + + if args.json: + print(json.dumps(findings.as_dict(), indent=2)) + else: + _report(findings) + if findings.total == 0: + print("Docs-tree gate OK — every referenced path exists, no stale line anchors.") + + if findings.total and not args.warn_only: + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_check_docs_tree.py b/scripts/test_check_docs_tree.py new file mode 100644 index 00000000..7d751351 --- /dev/null +++ b/scripts/test_check_docs_tree.py @@ -0,0 +1,114 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Unit tests for the D9 docs-as-checked-artifact gate (scripts/check_docs_tree.py). + +Two layers: + 1. Fixture-driven: build a throwaway doc + tree under tmp_path and assert + scan_docs() classifies each reference (exists / missing / removed-context / + generated / line-anchor) correctly. Hermetic. + 2. Self-guard: the REAL repo's inventory docs must pass the gate (main() == 0), + so any future drift fails this test too — not just the CI step. +""" +import importlib.util +import os +import sys + +HERE = os.path.dirname(os.path.abspath(__file__)) + + +def _load(): + spec = importlib.util.spec_from_file_location( + "check_docs_tree", os.path.join(HERE, "check_docs_tree.py") + ) + mod = importlib.util.module_from_spec(spec) + # Register before exec so the dataclass's `from __future__ import annotations` + # field resolution can find the module in sys.modules. + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +mod = _load() + + +def _write(root, rel, text): + p = root / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(text, encoding="utf-8") + return p + + +def test_existing_reference_passes(tmp_path): + _write(tmp_path, "expert_backend/services/foo.py", "x = 1\n") + _write(tmp_path, "doc.md", "See `services/foo.py` for details.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert f.total == 0 + + +def test_missing_reference_is_flagged(tmp_path): + _write(tmp_path, "doc.md", "See `services/ghost.py` for details.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert [r for _, _, r in f.missing_paths] == ["services/ghost.py"] + + +def test_removed_context_same_line_is_exempt(tmp_path): + _write(tmp_path, "doc.md", "The `recommenders/_service_integration.py` module was removed.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert f.missing_paths == [] + + +def test_removed_context_wrapped_line_is_exempt(tmp_path): + # The "former … removed" qualifier wraps onto neighbouring lines. + _write(tmp_path, "doc.md", + "the former\n`recommenders/_service_integration.py` module\nwas removed in D1.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert f.missing_paths == [] + + +def test_generated_artifact_is_exempt(tmp_path): + _write(tmp_path, "doc.md", + "Build produces `frontend/dist-standalone/standalone.html` and `Overflow_Graph/x.pdf`.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + # standalone.html (generated suffix) is exempt; Overflow_Graph/x.pdf has no + # tracked extension so it's not a candidate at all — either way, no finding. + assert f.missing_paths == [] + + +def test_line_number_anchor_is_flagged(tmp_path): + _write(tmp_path, "services/foo.py", "x = 1\n") + _write(tmp_path, "doc.md", "See the singleton at `services/foo.py:352` for details.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert [a for _, _, a in f.line_anchors] == ["services/foo.py:352"] + + +def test_line_range_anchor_is_flagged(tmp_path): + _write(tmp_path, "doc.md", "`App.tsx:310-324` resets state.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert [a for _, _, a in f.line_anchors] == ["App.tsx:310-324"] + + +def test_bare_filename_without_slash_is_ignored(tmp_path): + # Bare names (`main.py`, `App.tsx`) are illustrative and too ambiguous to + # resolve — only directory-qualified paths are existence-checked. + _write(tmp_path, "doc.md", "Edit `main.py` and `types.ts` as needed.\n") + f = mod.scan_docs(tmp_path, doc_files=["doc.md"]) + assert f.missing_paths == [] + + +def test_gate_returns_nonzero_only_without_warn_only(tmp_path, monkeypatch, capsys): + _write(tmp_path, "doc.md", "Broken `services/ghost.py`.\n") + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + monkeypatch.setattr(mod, "DOC_FILES", ["doc.md"]) + assert mod.main([]) == 1 + assert mod.main(["--warn-only"]) == 0 + out = capsys.readouterr().out + assert "services/ghost.py" in out + + +def test_real_repo_inventory_docs_are_clean(): + # Self-guard: the committed CLAUDE.md tree must stay drift-free, so a future + # rename that forgets the docs fails HERE, not only in the CI gate step. + assert mod.main([]) == 0 From 78a1c909a932403b96e7e7b857ffde69f09a1379 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 14:29:09 +0000 Subject: [PATCH 23/24] docs: track remaining deep-revision tails as in-repo issues (FU-5..FU-8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Issues are disabled on the fork, so followups.md is the canonical issue log. Add self-contained entries for the work that remains after the QW/FU-2/D9 pass, each pointing at its detailed tracking doc: - FU-5 — D2 tail: response models on the gzipped endpoints, types.ts generation from the OpenAPI snapshot, blanket-handler removal. - FU-6 — D7 tail: pinned Python-closure lockfile shared by CI + Dockerfile, config-merge on upgrade. - FU-7 — D6 tail: VL-count auto-enable of the bitmap pan/zoom engine (validation-gated on operator hardware). - FU-8 — decompose the size/complexity ceiling-riders (review target #4; unblocks FU-3/FU-4, which deferred on the exact-1150 recommender_service). Also marks FU-2 resolved (replay tooling landed). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- docs/architecture/followups.md | 125 +++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/docs/architecture/followups.md b/docs/architecture/followups.md index 237ef6bc..012fb64e 100644 --- a/docs/architecture/followups.md +++ b/docs/architecture/followups.md @@ -231,3 +231,128 @@ patch-vs-full branch before it ships, so it is not a drop-in quick win. - `expert_backend/services/diagram/action_patch.py` - `frontend/src/utils/svgPatch.ts` (`applyPatchToClone`), `frontend/src/utils/ndjsonStream.ts` (the shared reader) + +--- + +## FU-5 — D2 tail: finish the API-contract machine-check + +**Status:** open · **Area:** backend / contract · **Opened:** 2026-07-09 + +### Context + +D2 shipped the backbone: the unified `{detail, code}` error envelope +(`services/api_errors.py`), the committed `openapi.snapshot.json`, and the CI +diff (`scripts/check_openapi_contract.py` / `test_openapi_contract.py`). + +### What's deferred (and why) + +Three tail items remain, each additive and independently landable: + +1. **Response models on the gzipped endpoints.** The large diagram/analysis + payloads still return a bespoke `Response`, so only their *structure* is + snapshot-checked, not their response *shape*. Declare `response_model`s where + the payload is a small, stable, NumPy-free dict. +2. **Generate `types.ts` from `openapi.snapshot.json`.** The frontend interface + file is still hand-maintained against the snapshot; a generator would remove + the drift surface entirely. +3. **Remove the blanket exception handler** once every raise site uses the typed + `HTTPException` / `AppHTTPException` path, so no unexpected `str(e)` can leak. + +Deferred as polish once the higher-leverage structural revisions (D5/D6/D8/D9) +landed; none blocks a release. + +### References + +- [`api-contract-machine-check.md`](api-contract-machine-check.md) — the full + D2 plan and current state. +- `expert_backend/services/api_errors.py`, `expert_backend/openapi.snapshot.json`, + `frontend/src/types.ts`. + +--- + +## FU-6 — D7 tail: reproducible Python closure + Dockerfile config-merge + +**Status:** open · **Area:** delivery / reproducibility · **Opened:** 2026-07-09 + +### Context + +D7 landed the deployment-lockdown profile (`COSTUDY4GRID_LOCKDOWN`), the +test-gated HF deploy, and (with QW25) the Dockerfile hygiene pass (recommender +pin, `HEALTHCHECK`, real `PORT`, dead-weight trim, LFS-pointer validation on the +zipped grid). + +### What's deferred (and why) + +1. **Pinned Python-closure lockfile.** CI installs float within the `pyproject` + ranges, so "the Dockerfile mirrors CI" is only approximately true. A + `pip-compile` lockfile consumed by **both** CI and the Dockerfile would make + the deployed closure bit-for-bit reproducible. Deferred because it needs a + periodic-refresh workflow (a stale lock silently pins CVEs) — a process + commitment, not a one-shot edit. +2. **Config-merge on upgrade.** A new `config.default.json` key is not merged + into a user's existing on-disk config on upgrade; today it only seeds a + first run. Low urgency for the single-player Space. + +### References + +- [`deployment-trust.md`](deployment-trust.md) — D7 rationale + deploy flow. +- `Dockerfile`, `recommender-pin.txt`, `config.default.json`. + +--- + +## FU-7 — D6 tail: VL-count auto-enable of the bitmap pan/zoom engine + +**Status:** open · **Area:** frontend / perf · **Opened:** 2026-07-09 + +### Context + +D6 landed the SVG element-adoption pipeline (parse-once, adopt directly) and the +`'bitmap'` pan/zoom engine (`utils/svg/bitmapSnapshot.ts`) delivers ~3× fps on +the 5 k-VL grid, but it is **opt-in** (Settings → *Pan/zoom rendering*, default +`'off'`). + +### What's deferred (and why) + +Auto-selecting `'bitmap'` above a voltage-level threshold would give the win by +default, but the right threshold and the raster-fidelity trade-off (halo/delta +survival across the isolated raster, canvas-taint guards) need validation on +real operator hardware across grid sizes — not something to hardcode blind. Left +opt-in until that measurement exists. + +### Suggested approach + +Measure fps vs. VL count on representative hardware, pick a conservative +auto-enable threshold, and gate it behind the existing `smoothPanZoom` mode so +an operator can still force `'off'`. + +### References + +- `frontend/src/utils/svg/bitmapSnapshot.ts`, `frontend/src/utils/smoothPanZoom.ts` +- `docs/performance/history/interaction-paint-culling.md` + +--- + +## FU-8 — Decompose the size/complexity ceiling-riders (review target #4) + +**Status:** open · **Area:** backend + frontend / refactor · **Opened:** 2026-07-09 + +### Context + +Several modules ride their code-quality ceilings with a thin margin, so the next +feature PR that touches one trips the gate under deadline. This is structural +work that should be scheduled deliberately, not done reactively. + +### What's deferred (and why) + +The full investigation — every ceiling-rider with its current margin, a concrete +extraction candidate, a tightest-margin-first priority order, and acceptance +criteria — is already captured as a proposal. It is deferred (not abandoned) +because each extraction is a real refactor with its own regression surface; +`recommender_service.py` sitting at **exactly 1150** is what forced QW12/QW13 +(FU-3/FU-4) to defer, so this unblocks them. + +### References + +- [`../proposals/decompose-ceiling-riders.md`](../proposals/decompose-ceiling-riders.md) + — per-target margins, extraction candidates, priority order. +- [`code-quality-analysis.md`](code-quality-analysis.md) §22 — headline margins. From fc57569ca38a7e01a82742f2e5707e7f354380ca Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Thu, 9 Jul 2026 14:33:43 +0000 Subject: [PATCH 24/24] Release 0.9.0: consolidate 2026-07 review revisions + QW/FU-2/D9 Version - Bump pyproject.toml 0.8.0 -> 0.9.0; README release badge + "current release" line; root CLAUDE.md changelog pointer (0.8.0 -> 0.9.0). CHANGELOG - Promote [Unreleased] -> [0.9.0] (2026-07-09) with a release summary tying together the deep revisions D1-D8 that had accumulated under Unreleased. - Add this pass's work: backend perf QW11/17/6/22, frontend QW14/15/19/20/21, delivery/CI QW8/23/25, Game-Mode resilience QW24 + trusted replay (FU-2), and the docs-as-a-checked-artifact gate (D9). - Update the compare-link footer (0.9.0 tag + Unreleased base). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Am26qvtnHoY1qnmssaSQam Signed-off-by: Antoine Marot --- CHANGELOG.md | 87 +++++++++++++++++++++++++++++++++++++++++++++++++- CLAUDE.md | 2 +- README.md | 4 +-- pyproject.toml | 2 +- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f62f927d..f8ce0c1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,90 @@ and the project (informally) follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +## [0.9.0] — 2026-07-09 + +Release **0.9.0** consolidates the 2026-07 full-repo-review revisions — the +de-ghosted recommender subsystem (**D1**), the API-contract machine-check + +`{detail, code}` error envelope (**D2**), shared-`Network` concurrency ownership +(**D3**), the two frontend-hub relief stages (**D4**), the single streaming + +notification pipeline (**D5**), the SVG element-adoption pipeline (**D6**), +deployment trust & reproducibility (**D7**), and the reproducible-data + +benchmark supply chain (**D8**) — and adds a follow-up pass of quick wins, the +Game-Mode session-log replay (**FU-2**), and a new docs-as-a-checked-artifact +gate (**D9**). Remaining tails are tracked in +[`docs/architecture/followups.md`](docs/architecture/followups.md). + +### Backend performance & robustness (2026-07 quick wins) + +- **QW11 — vectorised `_diff_switches`.** The action-vs-contingency switch diff + ran a per-row `.loc` lookup (~85 k iterations on the France grid); it now uses + pandas `.isin` membership + `reindex(fill_value=False)` on boolean columns. + Guarded by `test_diff_switches.py`. +- **QW17 — single `Overflow_Graph` anchor + `reset()` completeness.** The output + directory is now one constant (`services/paths.py::OVERFLOW_DIR`) instead of + several ad-hoc joins, and every per-study cache added to + `RecommenderService` is swept by `test_reset_completeness.py` so a field that + is initialised but not cleared on reload fails a test (the class of leak that + bit `_layout_cache`). +- **QW6 — no path leaks in error details.** The filesystem / config endpoints + return a generic message with the real traceback `logger.exception`-logged, + instead of `str(e)` (which leaked absolute server paths). Complements the D2 + envelope. +- **QW22 — legacy-analysis watchdog.** The legacy `/api/run-analysis` PDF-poll + loop now has a deadline (`COSTUDY4GRID_ANALYSIS_TIMEOUT_S`, default 600 s) so a + stuck computation can no longer pin the worker forever. + +### Frontend (2026-07 quick wins) + +- **QW14 — highlight pipeline no longer re-runs on every pan/zoom settle.** The + driving effect's dependency array was narrowed from the whole `diagrams` + object to the container refs + metadata indices it actually reads. +- **QW15 — LRU-capped action-variant diagram cache** (`ACTION_DIAGRAM_CACHE_CAP`) + so the primed-diagram map can't grow unbounded across a long session. +- **QW19 — theme mirrored into detached popup windows**, kept in sync via a + `MutationObserver` on the host document's theme attribute. +- **QW20 — `useModalKeyboard`** centralises Escape-to-close, a focus trap, and + `role="dialog"` / `aria-modal`, wired into the confirmation, settings, and + reload-session modals. +- **QW21 — frontend `console.log` ceiling** added to the code-quality gate + (frozen at the current count and ratcheted down; D6's boost diagnostics were + deliberately kept). + +### Delivery & CI (2026-07 quick wins) + +- **QW8 — per-PR recommender pin.** `recommender-pin.txt` pins + `expert_op4grid_recommender` for the PR lanes so an upstream release can't + silently turn a green PR red; a weekly `canary.yml` floats to latest and flags + regressions early. +- **QW23 — single CI provider.** Removed `.circleci/config.yml`; GitHub Actions + is the sole pipeline. +- **QW25 — Dockerfile hygiene.** Recommender pin, `HEALTHCHECK`, real `PORT`, + dead-weight trim, and `scripts/extract_network_zip.py` that validates the + zipped France grid is a real archive (not an unresolved Git-LFS pointer) + before extraction. + +### Game Mode — resilience + trusted replay + +- **QW24 — mid-session recovery.** A study that fails to load no longer discards + the completed ones: the loading overlay offers **Retry**, **Finish with N + results**, and **Quit to setup**. A `presets ↔ overload-data` consistency test + fails fast if a regenerated grid drops a preset contingency. +- **FU-2 — physically replayable session log.** `e2e_game_session.py --replay` + re-derives trusted `finalMaxRho` / `solved` from a session log's recorded + actions by re-driving the backend, writes a `reference.json` the Codabench + scorer consumes, and flags divergence from self-reported numbers. Hermetic + coverage in `scripts/game_mode/test_replay.py`. + +### Docs as a checked artifact (D9) + +- **`scripts/check_docs_tree.py`** gates the hand-maintained `CLAUDE.md` + inventory: every directory-qualified path reference must resolve to a real file + (with generated-artifact and referenced-as-removed exemptions), and rotting + `file.py:NNN` line anchors are forbidden in favour of symbol anchors. The seven + pre-existing stale anchors were converted; unit coverage + a real-repo + self-guard live in `scripts/test_check_docs_tree.py`. See + `docs/architecture/code-quality-analysis.md` §23. + ### Tests + docs — coverage and reference for the 2026-07 deep revisions - **New `test_api_errors.py`** (D2): direct coverage of the error envelope — @@ -1701,7 +1785,8 @@ the authoritative reference for pre-0.5.0 work. --- -[Unreleased]: https://github.com/marota/Co-Study4Grid/compare/0.8.0...HEAD +[Unreleased]: https://github.com/marota/Co-Study4Grid/compare/0.9.0...HEAD +[0.9.0]: https://github.com/marota/Co-Study4Grid/releases/tag/0.9.0 [0.8.0]: https://github.com/marota/Co-Study4Grid/releases/tag/0.8.0 [0.7.5]: https://github.com/marota/Co-Study4Grid/releases/tag/0.7.5 [0.7.0]: https://github.com/marota/Co-Study4Grid/releases/tag/0.7.0 diff --git a/CLAUDE.md b/CLAUDE.md index 3dcd619b..ac60cf68 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ Co-Study4Grid is a full-stack web application for **power grid contingency analy Co-Study4Grid/ ├── CLAUDE.md # This file — project overview + standalone parity audit ├── README.md # User-facing project description + quick start -├── CHANGELOG.md # Per-release changelog (current: 0.8.0) +├── CHANGELOG.md # Per-release changelog (current: 0.9.0) ├── CONTRIBUTING.md # Contributor setup, code-quality gate ├── pyproject.toml # Python project metadata + ruff config (E9/F ruleset) ├── pytest.ini # Pytest config (testpaths = expert_backend/tests) diff --git a/README.md b/README.md index 53ec1aa6..f2418e04 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ different model. See [Plug Your Own Recommendation Model](#plug-your-own-recomme to extend it. ![License: MPL 2.0](https://img.shields.io/badge/license-MPL--2.0-blue) -![Release](https://img.shields.io/badge/release-0.8.0-green) +![Release](https://img.shields.io/badge/release-0.9.0-green) --- @@ -887,7 +887,7 @@ layers of automated checks (`scripts/check_standalone_parity.py`, ## Changelog -See [`CHANGELOG.md`](CHANGELOG.md) for the list of changes per release. The current release is **0.8.0**. +See [`CHANGELOG.md`](CHANGELOG.md) for the list of changes per release. The current release is **0.9.0**. --- diff --git a/pyproject.toml b/pyproject.toml index 687e911b..fe2f40ed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "co_study4grid" -version = "0.8.0" +version = "0.9.0" authors = [ { name="RTE", email="rte@rte-france.com" }, ]