Release 0.2.6 — reassessment performance, security & robustness fixes, CI migration#111
Merged
marota merged 14 commits intoJul 3, 2026
Merged
Conversation
…perf, maintainability) Six-dimension audit (architecture/orchestration, action_evaluation, pypowsybl backend & performance, graph_analysis/utils, manoeuvre/IHM, tests/docs/CI/packaging) cross-checked with radon/ruff metrics. Headline findings: pypowsybl rho-check baseline mix-up in discovery, IHM /api/load_scenario path traversal, MultiDiGraph edge-cache collapse on parallel circuits, unbounded kept-variant leak, sys.exit(0) in library code. Includes prioritized deep-revision roadmap and quick wins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
…ency state check_rho_reduction (pypowsybl) passed the healthy N-state observation to check_rho_reduction_with_baseline, which simulates only the candidate action on top of it — so topological-mixin candidates were compared against an N-1 baseline while being simulated on the healthy grid. Pass obs_baseline (the contingency-applied kept-variant observation already computed) instead, matching the grid2op contract. Impact was diagnostic-only (effective/ineffective labels, gated by CHECK_ACTION_SIMULATION); scores, ranking, returned actions and the reassessment rho numbers were unaffected. Also correct the review doc: downgrade the finding from Correctness to cosmetic with the traced data-flow rationale, note the injection-mixin path folded into revision R4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
The five topological discovery passes (reconnection / disconnection / merge / split / PST) called the full check_rho_reduction per candidate, which re-ran the N-1 baseline load flow and minted one keep_variant=True copy of the N-1 state per candidate that was never released (~2x load flows and ~30 orphaned variants per analysis, all duplicates of the already-kept N-1 variant). The intentionally-retained N / N-1 / prioritized-action variants are untouched. Route those passes through the single cached baseline (_get_simulation_baseline + check_rho_reduction_with_baseline), wired in the main.py pypowsybl block so the grid2op path and the mock-based discovery tests stay byte-identical. Extend _get_simulation_baseline to expose the baseline observation and a backend-aware branch point: grid2op re-applies the contingency and branches from the N-state, pypowsybl branches from the contingency-applied kept variant. The three injection passes now branch from that same shared baseline, closing the injection-path remnant of the earlier rho-check contract fix. Result: one baseline load flow and one kept baseline variant per run instead of per candidate. Review doc updated: variant-leak finding reframed (the retained states are intentional; the defect was the per-candidate duplicate) and marked fixed, P1 marked fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
Faithful translation of the two CircleCI jobs into a single GHA workflow: - build-and-test: pip install requirements.txt + numpy, apply the pypowsybl2grid patch, run 'pytest -m "not slow"' (Python 3.12). - quality: ruff check . (whole repo, blocking), interrogate on the manoeuvre module (>=80%, blocking), radon complexity (informational, non-blocking via continue-on-error). Runs on push to main, every pull_request, and manual dispatch. Removes .circleci/config.yml and updates the CLAUDE.md / manoeuvre-docs references that pointed at the CircleCI job. This adds the pytest job GitHub Actions was missing; the existing code-quality.yml (report + strict TODO/hardcoded-path gate) is retained. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
The overflow graph is a MultiDiGraph, but _get_edge_data_cache keyed its name/label lookup dicts by (u, v) 2-tuples, so parallel circuits between the same two substations (twin RTE lines, e.g. L61/L62) overwrote each other — only the last survived. _build_node_flow_cache then dropped one of every parallel pair from the load-shedding / curtailment / redispatch flow-influence scoring, and its behavior differed depending on whether the collapsed cache or the 3-tuple-keyed nx.get_edge_attributes fallback was used. Key both lookup dicts by the full edge id (u, v, k) on a multigraph (falling back to (u, v) on a plain graph), built in a single pass. _build_node_flow_cache reads only edge[0]/edge[1], so it is unaffected by the wider key. Adds tests/test_parallel_circuit_flow_cache.py: two parallel -50 MW circuits now sum to 100 MW at the shared node instead of collapsing to 50. Review doc: C3 marked fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
…ard bugs
manoeuvre IHM (scripts/manoeuvre_ihm.py):
- Path traversal: /api/load_scenario read (SCEN_DIR / f'{name}.json') with an
unsanitized client name. Add _stored_json_path() (sanitize via _safe_name +
require the resolved parent to equal the base dir); use it on the load and
the meta re-read; api_load_scenario returns 400/404 instead of reading an
arbitrary file. Verified: ../secret, ../../etc/passwd, /etc/passwd, foo/../x
are all contained or rejected.
- /api/config could repoint SCEN_DIR/SEQ_DIR anywhere (→ zip-archive
exfiltration + /api/save arbitrary write). Add _dir_within_allowed() and only
accept dirs under the cwd or MANOEUVRE_DATA_DIR/DGITT_CACHE_DIR; log accept/refuse.
Library sys.exit(0) -> exceptions:
- environment.py / environment_pypowsybl.py raised the process down with a
SUCCESS code on DC-fallback divergence. Add exceptions.LoadFlowDivergedError
(subclasses RuntimeError, so the CLI's except (ValueError, RuntimeError,
TypeError) still maps it to exit 1) and raise it instead; drop the now-unused
import sys.
utils hard bugs:
- load_training_data.set_state: state = action_path[0] on a StateInfo (no
__getitem__) -> state = action_path.
- filter_out_non_reproductible_observation used line_we_disconnect, a global
that only existed in __main__ (NameError when imported) -> now a parameter;
caller updated.
- load_evaluation_data: raise('string') -> raise ValueError(...).
- run_remedial_action: the 'reconnect maintenance' action set_bus -1
(disconnect) -> set_line_status 1 (reconnect), matching
run_contingency_on_scenario; collect the still-overloaded timesteps into a
fresh list so the return value and 'Success' print match the docstring.
Review doc: C2/C5/C6 marked fixed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg
Signed-off-by: Antoine Marot <amarot91@gmail.com>
The shared-baseline / branch-obs rework (4b3eb21), the C3 MultiDiGraph edge-cache change (a9cf9a1), and the earlier rho-check obs_baseline tweak (92cff0a) regressed the recommendation set on the config_pypsa_eur_* / pypowsybl configuration: 5 fewer suggestions (the non-topological injection actions and one parallel-line disconnection disco_way_196544700_b-220 disappeared), while shared actions kept identical max-loading. Restore the discovery pipeline byte-for-byte to main (401f25a): _base.py, _load_shedding.py, _redispatch.py, _renewable_curtailment.py, main.py, utils/simulation_pypowsybl.py; drop the parallel-circuit regression test. Verified: git diff vs 401f25a over action_evaluation/, main.py and simulation_pypowsybl.py is empty; ruff clean; discovery unit tests pass. Kept (non-discovery, no effect on recommendations): C2 IHM path-traversal fix, C5 sys.exit->LoadFlowDivergedError, C6 utils bug fixes, the CI migration, and the review document (annotated with a revert status note). The reverted fixes remain valid in principle and need safe re-introduction validated against the pypsa_eur config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
The suspected recommendation regression on config_pypsa_eur_* was a bad local config; with the correct config the recommendation set is identical to main. Restore the shared-baseline (4b3eb21), C3 edge-cache (a9cf9a1) and rho-check obs_baseline (92cff0a) changes and the parallel-circuit test. Review doc status note updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
Instruments PypowsyblObservation.simulate and attributes each load flow to its pipeline phase (step1 / graph / discovery / reassessment), printing call counts and wall time per phase so the reassessment cost is visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
…n re-simulation Benchmark (Co-Study4Grid scenario 1: contingency relation_8423570-225 / LANNEL61PRAGN -> MARSIL61PRAGN overload, CHECK_ACTION_SIMULATION=False, the game config) showed reassessment dominates: ~24s of a ~32s run, doing one full simulate() (load flow + observation build) per prioritized action; discovery does no candidate simulation in this config, so the earlier shared-baseline work is inert here. A) Observation construction (observation.py._refresh_state): remove redundant cross-JNI DataFrame fetches — get_buses x3 -> x1 (fetch connected_component in the same call), get_loads x2 -> x1, and cache the variant-invariant R/X on the NetworkManager instead of re-fetching get_lines()/get_2wt() per observation. Observation build ~0.47s -> ~0.25s (halved). B) Parallelise the per-action re-simulation across worker threads. pypowsybl releases the GIL during the load flow but the working variant is network- global, so each worker owns a private network copy (cloned via save/load_from_binary_buffer from the N-1 baseline variant, so no per-worker contingency load flow). Workers = min(10, cpu_count, n_actions). Results are bit-identical to serial; robust fallback to serial on any error. The worker count / cores used are surfaced in result['reassessment_parallelism'] and printed, so the UI can annotate the reassessment-time tooltip. Measured (4-core host, scenario 1, 15 actions): reassessment 16.4s -> 11.4s (1.43x); larger gains expected on higher-core hosts (up to the 10-worker cap). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
Bump version 0.2.5 -> 0.2.6. CHANGELOG [0.2.6] section + docs/release-notes/ v0.2.6.md summarising the reassessment performance work (parallel per-action re-simulation + observation-fetch dedup / R/X cache), the security & robustness fixes (IHM path traversal, sys.exit -> LoadFlowDivergedError, utils bugs, parallel-circuit edge cache), the shared discovery baseline, and the CircleCI -> GitHub Actions migration. Review doc annotated with an 'what was implemented' summary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
M1 — CLAUDE.md was a full minor version stale. Rewrite the architecture tree (discovery/ package with per-family mixins, models/, manoeuvre/, exceptions.py, antenna_graph.py, reassessment.py), drop the removed ActionType enum reference, bump the version/highlights to 0.2.6, document the new config keys (ALLOWED_ACTION_TYPES, ENABLE_ANTENNA_RECOMMENDATIONS, MIN_REDISPATCH, REDISPATCH_*), the discovery-package Key Files mapping, the star-import + package-attribute config-override gotcha, and the new reassessment_parallelism result key. M2 — the test config-override was fragile by construction: config_test.py was a hand-maintained fork of config.py that silently drifted (missing keys surfaced as AttributeError deep in runs). It now star-imports the real config (running pydantic Settings validation) and overrides only the test deltas. conftest.py keeps the package attribute in sync (_pkg.config = config_test) so `from expert_op4grid_recommender import config` resolves to the test module despite the star-import. Added a regression-guard test asserting the swapped-in config exposes the real config's keys while the DO_VISUALIZATION override still wins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
marota
force-pushed
the
claude/repo-architecture-review-yrhkn0
branch
from
16d9290 to
67e4b1e
Compare
The test config-override now star-imports the real config.py (review M2), which imports pydantic_settings. requirements.txt — the dependency list the CI test job installs from — was missing pydantic/pydantic-settings, so the CI run failed at conftest import with ModuleNotFoundError. They are genuine runtime dependencies of config.py (already declared in pyproject.toml); add them to requirements.txt so the CI environment installs them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X7hJ97MQ5mQ4x7LCxN4dBg Signed-off-by: Antoine Marot <amarot91@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v0.2.6 — Reassessment performance (parallel + observation dedup), security & robustness fixes, CI migration
A feature/bug-fix release on top of
0.2.5. Headline: the end-of-run actionreassessment — the dominant cost of an analysis on large networks — is now
parallelized and the per-observation cost is roughly halved. Plus a batch of
security / robustness fixes surfaced by a full-repository review, and a CI
migration from CircleCI to GitHub Actions.
Highlights
Performance — reassessment
Benchmarking the pypowsybl pipeline on the pan-European pypsa-eur grid
(Co-Study scenario 1: contingency
relation_8423570-225/ LANNEL61PRAGN →MARSIL61PRAGN overload,
CHECK_ACTION_SIMULATION=False— the game config)showed the per-action reassessment dominates the run (~24 s of ~32 s), doing
one full
simulate()(load flow + observation build) per prioritized action,while discovery runs no candidate simulation in that config.
threads, each on its own pypowsybl network copy (cloned via
save/load_from_binary_bufferfrom the N-1 baseline variant, so noper-worker contingency load flow). pypowsybl releases the GIL during the load
flow, but the working variant is network-global, so per-worker private
networks are required for correctness. Workers =
min(10, cpu_count, n_actions). Results are bit-identical to the serial path, with a robustfallback to serial on any error. The worker/core count used is surfaced in
result["reassessment_parallelism"](and printed) so the UI can annotate thereassessment-time tooltip. Measured ~1.43× on a 4-core host (15 actions);
larger gains on higher-core hosts.
fetches in
PypowsyblObservation._refresh_state(get_buses×3→×1,get_loads×2→×1) and cached the variant-invariant line R/X on theNetworkManagerinstead of re-fetchingget_lines()/get_2wt()on everyobservation. Observation build ~0.47 → ~0.25 s.
CHECK_ACTION_SIMULATIONis on, thetopological discovery passes share a single contingency baseline load flow
instead of recomputing it per candidate — halving discovery load flows and
removing a per-candidate kept-variant leak.
Fixed
/api/load_scenariosanitizes and confinesthe client-supplied scenario name to the scenarios directory (HTTP 400/404 on
invalid names);
/api/configonly accepts directories under an allowed root,closing a zip-archive exfiltration / arbitrary-write vector.
sys.exit(0)in library code. Load-flow divergence (including theDC fallback) now raises
LoadFlowDivergedError(aRuntimeErrorsubclass theCLI already maps to a non-zero exit) instead of terminating the host process
with a success code.
utils/load_training_data.py/load_evaluation_data.py. Fixed aStateInfoindexingTypeError, aNameErroron a__main__-only global, araise "string", and a maintenance "reconnect" action that actuallydisconnected the lines.
(u, v, key)id soparallel circuits are no longer collapsed in the load-shedding / curtailment /
redispatch flow-influence scoring (regression test added).
Changed
.github/workflows/ci.yml):a faithful port of the
build-and-test(pytest + the pypowsybl2grid patch)and
quality(ruff / interrogate / radon) jobs;.circleci/removed.Added
scripts/benchmark_pipeline.py— per-phase load-flow benchmark of thepypowsybl pipeline (load flows and wall time per phase, reassessment included).
docs/reviews/2026-07_full_code_review.md— a comprehensive architecture /interface / performance / maintainability review of the repository.
Upgrade notes
No breaking API changes.
run_analysis(...)gains areassessment_parallelismkey in its result dict; existing keys are unchanged. Reassessment parallelism is
automatic (bounded by
min(10, cpu_count, n_actions)) and falls back to serialtransparently.