Skip to content

Fix action re-simulation and overload highlighting bugs - #83

Merged
marota merged 12 commits into
mainfrom
claude/sync-action-target-mw-BN4a1
Apr 13, 2026
Merged

marota merged 12 commits into
mainfrom
claude/sync-action-target-mw-BN4a1

Conversation

@marota

@marota marota commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes five critical bugs related to action re-simulation, overload highlighting, settings confirmation, and network path changes. The changes ensure that re-simulating actions preserves their bucket assignment, overload highlights display correctly across different tabs, and user confirmations are properly gated for destructive operations.

Key Changes

Bug 1: Target MW/Curtailment Sync for Computed Actions

  • Renamed scoreTargetMw → cardEditMw and onScoreTargetMwChange → onCardEditMwChange for clarity
  • Fixed ActionSearchDropdown to populate score-table inputs with stored shedded_mw (load-shedding) and curtailed_mw (renewable curtailment) values from computed actions
  • Ensures users see the simulated values that actions were run with, not empty inputs

Bug 2: SLD Overload Highlight Regression

  • Fixed SldOverlay to highlight only post-action overloads on the ACTION tab (from lines_overloaded_after), not stale N-1 overloads
  • Uses useLayoutEffect with signature-based guards to detect when highlight clones are wiped out and re-apply them
  • Ensures persistent overloads (present in both N-1 and post-action states) are correctly highlighted

Bug 3: Action Re-fetch on Force Select

  • Added force flag to handleActionSelect to bypass the "same id → toggle off" early return
  • Ensures re-simulating an already-selected action re-fetches its diagram instead of silently deselecting it
  • Wrapped as wrappedForcedActionSelect for use after re-simulation

Bug 4: File Picker Error Surfacing

  • Modified useSettings.pickSettingsPath to surface backend errors via window.alert when the native file picker fails
  • Prevents silent failures when tkinter/display is unavailable
  • Updated main.py to use a tiny topmost root window instead of withdraw() to ensure dialogs appear on top

Bug 5: Action Re-simulation Bucket Preservation

  • Added handleActionResimulated hook to update actions in place without promoting them to Selected Actions
  • Renamed wrappedManualActionAdded → wrappedForcedActionSelect to clarify it's for new manual actions
  • Separated re-simulation flow from manual action addition:
    • Manual actions use onManualActionAdded (promotes to Selected)
    • Re-simulated actions use onActionResimulated (stays in current bucket)
  • Updated ActionFeed and CombinedActionsModal to dispatch through correct callbacks
  • Added backend logic in _enrich_actions to compute lines_overloaded_after when missing

Settings & Network Path Confirmation

  • Added confirmation dialogs for:
    • Applying settings when analysis state exists (prevents accidental loss of work)
    • Changing network path while a study is loaded
  • Introduced committedNetworkPathRef to track the network path of the currently-loaded study
  • Settings apply immediately when no analysis exists, but require confirmation otherwise
  • Network path changes are gated by confirmation when a study is already loaded

CombinedActionsModal Estimation Card Persistence

  • Fixed regression where the Explore Pairs estimation/comparison card disappeared after Simulate Combined
  • Changed useEffect dependency to exclude analysisResult so the card stays visible when the parent mutates it
  • Card now only resets when user changes pair selection or leaves the Explore Pairs tab

NAD Highlight Layering Fix

  • Fixed applyActionTargetHighlights to only remove its own .nad-action-target clones, not all .nad-highlight-clone elements
  • Preserves .nad-overloaded clones planted by applyOverloadedHighlights
  • Ensures Remedial Action tab shows orange halos for persistent/new overloads

CSS & Visual Consistency

  • Updated SLD overload highlight to use drop-shadow halo (consistent with contingency style) instead of dashed stroke
  • Changed color to #ff8c00 (orange) for visual distinction

Testing

  • Added comprehensive test coverage for all five bugs

https://claude.ai/code/session_01V2CpYAAVQfoy8q8vq9jTMk

claude added 12 commits April 12, 2026 19:56
…ir sim, settings pickers

- ActionSearchDropdown: share cardEditMw state with the action card so the
  Target MW input for LS/RC rows defaults to the simulated shedded_mw /
  curtailed_mw and stays in sync between the score table row and the
  prioritized action card. Re-simulation is only enabled once the user
  actually changes the value.
- SldOverlay: on the ACTION SLD tab, highlight post-action overloads from
  actionDetail.lines_overloaded_after instead of stale N-1 overloads, so
  overloads solved by the action are no longer highlighted and any that
  persist or newly appear are shown. Manual-action and combined-pair
  simulation results now propagate lines_overloaded_after into App state.
- CombinedActionsModal / useDiagrams / App: after a (re)simulation from the
  Explore Pairs tab the modal now closes automatically and handleActionSelect
  can be called with force=true so the action-variant diagram is always
  re-fetched (fixes blank diagram after simulating a pair and when clicking
  other action cards afterwards).
- Settings path pickers: surface backend errors in the UI via a visible
  alert, and make the tkinter subprocess more reliable (foregrounded root
  window, stderr capture, timeout) so failures no longer look like dead
  buttons.
- Re-simulating an already-listed action (editing Target MW / PST tap on
  a card and clicking Re-simulate) no longer silently promotes it into
  Selected Actions. A new handleActionResimulated updates the action
  detail in place while preserving its existing is_manual flag and
  leaving selectedActionIds / manuallyAddedIds untouched, so a
  recommender-suggested card stays in Suggested Actions.
- Wired wrappedActionResimulated through App → ActionFeed, and
  handleResimulate / handleResimulateTap now use it instead of
  onManualActionAdded.
- CombinedActionsModal: simulations (single row from Explore Pairs,
  green Simulate Combined, or Computed Pairs row) no longer auto-close
  the modal. Single-action simulations go to Suggested Actions via the
  new onSimulateSingleAction callback, combined pairs still get promoted
  to Selected. The action card and action-variant diagram load in the
  background so the user can keep interacting with the modal.
Covers the seven bugs fixed in this branch:

- Bug 1 (Target MW sync): ActionSearchDropdown.test.tsx — LS/RC score
  table rows default to the stored shedded_mw / curtailed_mw for
  computed actions, cardEditMw overrides the default, typing forwards
  through onCardEditMwChange, and non-computed rows stay empty.
- Bug 2 (Post-action SLD overload highlight): SldOverlay.test.tsx —
  on the ACTION tab the highlight effect uses
  actionDetail.lines_overloaded_after (not stale N-1 overloads), N-1
  tab still highlights result.lines_overloaded, and an action that
  resolves every overload produces zero highlight clones.
- Bug 3 (Blank action diagram on re-select): useDiagrams.test.ts —
  handleActionSelect(force=true) refetches the variant diagram for the
  already-selected action instead of deselecting, and the default
  toggle-off path is preserved.
- Bug 4 (Settings path picker error surfacing): useSettings.test.ts
  + api.test.ts — pickSettingsPath now calls window.alert with the
  backend error, api.pickPath throws when the backend returns an
  error field, and the success path is unchanged.
- Bug 5 (Resimulate preserves bucket): useActions.test.ts — new
  handleActionResimulated does not add to selectedActionIds /
  manuallyAddedIds, preserves is_manual=true where it existed, still
  logs manual_action_simulated, and ActionFeed.test.tsx verifies LS
  and PST re-simulate buttons dispatch through onActionResimulated and
  never through onManualActionAdded.
- Bugs 6 & 7 (Combination modal dispatch & persistence):
  CombinedActionsModal.test.tsx — single-action Explore Pairs
  simulation routes through onSimulateSingleAction, computed-pair
  simulation routes through onSimulateCombined, and neither path
  calls onClose (modal stays open).

Total: 21 new assertions, all 599 frontend tests pass.
The combination modal was capped at a fixed 950px width, which was
narrower than the Computed / Explore Pairs tables on most screens
and produced a horizontal scrollbar at the modal level — users had
to scroll left/right to see all columns.

- Card width is now 95vw (max 95vw) so the dialog uses (almost) the
  full viewport on any screen size, and the inner tables have room
  to lay out without overflowing.
- The body container now sets overflowX: 'hidden' and minWidth: 0
  so any over-wide inner element scrolls within its own sub-
  container instead of leaking out to the modal level.
- Added `data-testid` hooks on the modal card and body for the
  layout regression tests.

Tests (CombinedActionsModal.test.tsx → "modal layout width"):
- width / maxWidth are both "95vw" (regression guard: not "950px")
- body overflowX is hidden, overflowY is auto, minWidth is 0
- outer modal card still hides its own overflow
Clicking "Simulate Combined" on the estimation/comparison card used
to make the card disappear as soon as the simulation finished. The
useEffect driving the card re-ran whenever analysisResult changed,
and because onSimulateCombined writes the new pair into
analysisResult.actions (not combined_actions) the "preComputed"
lookup returned undefined and setPreview(null) hid the card.

- Remove analysisResult from the preview useEffect dependency array
  so mutations to analysisResult.actions during a simulation no
  longer clear the card the user is currently reading.
- The card is still reset when the user changes their pair
  selection or leaves the Explore Pairs tab (both paths go through
  handleToggle / tab switch which re-run the effect with the new
  selectedIds or activeTab).

Tests (CombinedActionsModal.test.tsx → new "Estimation card persistence"):
- Card stays visible and the simulation feedback is shown after
  Simulate Combined completes, even after the parent re-renders
  with an analysisResult that now contains the simulated pair in
  `actions`.
- Card resets when the user deselects one of the two actions.
- Card resets when the user switches back to the Computed Pairs tab.
Two related SLD overlay issues:

1. The overload highlight used a dashed orange stroke that was
   visually inconsistent with the other halo-style highlights
   (contingency, action, breaker) and was easy to lose on dense
   SLDs.
2. When the user panned or zoomed the SLD overlay, the highlight
   clones imperatively planted into the SVG disappeared. They only
   came back after a tab switch re-triggered the useEffect. Root
   cause: React's reconciliation of the <div
   dangerouslySetInnerHTML> wrapper can drop the clone siblings on
   re-render, and the old useEffect only re-applied highlights
   when a dep in its dep array changed — pan just mutates a local
   transform state, so the effect never re-fired.

Fix:

- App.css: overload highlight is now a drop-shadow halo with
  stroke: #ff8c00 (solid, 6px) and filter: drop-shadow(...)
  matching the contingency style. Rect/circle are also covered
  so bus nodes get a halo too.
- SldOverlay.tsx: the highlight effect is now a useLayoutEffect
  with no dep array. It self-gates via a `appliedSigRef` signature
  and a DOM check: if the signature matches the last applied set
  AND the clones are still in the DOM, skip; otherwise re-plant.
  This makes the effect idempotent (no duplication on normal
  re-renders) while guaranteeing clones are replanted whenever the
  DOM has lost them.

Tests:

- cssRegression.test.ts:
  * overload clone uses drop-shadow(#ff8c00) twice (halo)
  * stroke is solid #ff8c00 6px, stroke-dasharray: none
    (regression guards against 5px / `6 3` dash pattern)
  * rule covers path / line / polyline / rect / circle
- SldOverlay.test.tsx ("highlight persistence across pan..."):
  * replants highlight clones after a simulated pan reconciliation
    that wiped them out of the DOM
  * idempotent: repeated rerenders with unchanged inputs do not
    stack duplicate clones
…ighlights survive

The discovery engine does NOT populate lines_overloaded_after on
recommender-suggested actions, so every suggested action reached
the frontend with lines_overloaded_after == []. The SLD overlay's
ACTION tab (and the main NAD Action tab, which first tries the
same field) therefore showed zero overload highlights even for
actions that left an overload in place or created a new one on a
different line — a regression the user noticed when staring at a
full Action-tab diagram with no orange halos.

Fix (backend):

- `_enrich_actions` now accepts an optional `lines_overloaded_names`
  parameter (the ordered N-1 overloaded line list that already
  lives in `results["lines_overloaded_names"]`). When the engine
  has not provided `lines_overloaded_after`, it is computed as:
    * every N-1 overloaded line whose raw rho_after[i] >= 1.0, AND
    * max_rho_line when its raw max_rho >= 1.0 (captures brand-new
      overloads outside the N-1 monitoring set).
  If the engine already populated the list, it is preserved
  verbatim (simulated actions coming through simulate_manual_action
  already carry authoritative values).
- Both `_enrich_actions` call sites in `analysis_mixin.py` pass
  `lines_overloaded_names=` so suggested actions in run_analysis
  and run_analysis_step2 both get the computed field.

Tests (backend):

- `test_enrich_actions_computes_lines_overloaded_after_when_missing`
  — 3 scenarios: action still overloaded (persistent), action
  creates a new overload elsewhere, action solves everything.
- `test_enrich_actions_preserves_existing_lines_overloaded_after`
  — back-compat for actions that already carry a library-
  provided list.
- `test_enrich_actions_without_lines_overloaded_names_defaults_to_max_rho_line_only`
  — legacy call sites fall back to max_rho_line when max_rho >= 1.0.
- `test_recommender_filtering.py::mock_enrich` updated to accept the
  new kwarg.

Tests (frontend, SldOverlay.test.tsx → post-action overload
highlight block):

- persistent overload: line is in BOTH N-1 overloads AND
  lines_overloaded_after → highlighted on the ACTION tab.
- mix of persistent + brand-new overloads → both highlighted,
  resolved overload NOT highlighted.
Root cause of the "no overload halos on the Remedial Action tab"
regression: applyHighlightsForTab('action') calls
applyOverloadedHighlights first (planting .nad-overloaded clones in
the background layer) and then applyActionTargetHighlights right
after. The latter was blanket-removing every .nad-highlight-clone
element in the container, which wiped the freshly-planted overload
halos on every render. The SLD overlay was unaffected because it
uses a separate highlight pipeline.

Fix:

- Use a compound selector (`.nad-highlight-clone.nad-action-target`)
  so only our own action-target clones are removed; .nad-overloaded
  clones in the background layer survive.
- Remove the clones BEFORE stripping the `nad-action-target` class
  from originals. Without this ordering, the class-strip would also
  clear `nad-action-target` from the clones (they carry it too) and
  the follow-up selector could no longer find them.

Tests (svgUtils.test.ts → describe applyActionTargetHighlights):

- preserves existing .nad-overloaded clones when re-applying action
  target highlights — regression guard that directly exercises the
  bug: fixture has one .nad-overloaded clone and one stale
  .nad-action-target clone; after the call the overload clone must
  still be in the DOM, the stale action-target clone must be gone,
  and a new action-target clone must have been planted for L1.
- preserves overload clones when called with null actionDetail —
  the "deselect action" path must still scrub action-target clones
  but leave overload clones intact.
Two related regressions on the Network (N-1) and Remedial Action
NAD tabs in Impacts ("delta") mode:

- The contingency halo disappeared.
- N-1 overload halos disappeared.

Root cause: applyDeltaVisuals tags ORIGINAL svg elements with
.nad-delta-positive / .nad-delta-negative / .nad-delta-grey. The
clone-based highlight functions then call cloneNode(true) on those
tagged originals; the clones inherit the delta class, and because
the .nad-delta-* CSS rules are declared LATER in App.css than
.nad-overloaded / .nad-action-target / .nad-contingency-highlight,
they win the cascade and turn the 150px coloured halo into a 3px
delta-coloured stroke — visually making the halo disappear. On top
of that, applyHighlightsForTab explicitly skipped
applyOverloadedHighlights when actionViewMode === 'delta', so on
the N-1 tab the halos were not even refreshed in Impacts mode.

Fix:

- svgUtils.ts: applyOverloadedHighlights, applyContingencyHighlight
  and applyActionTargetHighlights now strip
  nad-delta-positive / nad-delta-negative / nad-delta-grey from each
  clone immediately after cloneNode(true), so the halo CSS always
  wins the cascade regardless of the original's current delta class.
- App.tsx applyHighlightsForTab:
  * Reordered both tabs so the clone-based highlight functions run
    BEFORE applyDeltaVisuals (defense in depth — clones are now
    captured from pristine elements).
  * Removed the actionViewMode !== 'delta' guards so overload halos
    are refreshed in BOTH Flows and Impacts modes. The user looks at
    Impacts to see how the action redistributes flows AND which
    lines are still / newly overloaded; suppressing the halos there
    hides exactly that information.

Tests (svgUtils.test.ts → describe "Highlight clones strip
nad-delta-* classes (Impacts mode regression)"):

- applyOverloadedHighlights clones are free of nad-delta-* classes
  even when the source elements carry them.
- applyContingencyHighlight clone is free of nad-delta-* classes
  even when the source element carries one.
- applyActionTargetHighlights clones are free of nad-delta-* classes
  even when the source element carries one.
- App.css ordering check: .nad-delta-* declarations come AFTER the
  highlight-rule declarations, so the strip dance above is genuinely
  required (regression guard against future CSS reordering).
When a study was already loaded and the user applied a different
config (or any settings change that requires reloading the network)
all results, manual simulations, action selections and diagrams were
silently wiped — the same cost the Load Study button is gated behind
with a confirmation dialog. Apply Settings now goes through the same
dialog.

Fix:

- ConfirmationDialog: new 'applySettings' state with copy
  ("Apply New Settings?" + "The network will be reloaded with the new
  configuration."). Each variant now also carries a stable
  `data-testid="confirm-dialog-<type>"` for tests.
- App.tsx:
  * Renamed the existing handleApplySettings to applySettingsImmediate
    (no behavior change beyond the name).
  * New handleApplySettingsClick gates on hasAnalysisState() and
    routes through setConfirmDialog({ type: 'applySettings' }) when
    a study is already in progress; otherwise it applies immediately.
  * handleConfirmDialog dispatches the new 'applySettings' branch.
  * The SettingsModal is now wired to handleApplySettingsClick.

Tests:

- ConfirmationDialog.test.tsx: new "renders correctly for apply
  settings" test asserting title, body, and stable testid.
- App.session.test.tsx: new "Apply Settings Confirmation" describe
  block with four scenarios:
    * applies settings directly when no analysis state
    * shows confirmation dialog with the right copy after running
      analysis (and does NOT call updateConfig until confirmed)
    * proceeds with apply on Confirm (dialog dismissed, modal closed,
      backend called)
    * cancels: dialog dismissed, no backend call, settings modal
      stays open and the contingency selection survives intact
- The pre-existing "clears branch and analysis state after Apply
  Settings with analysis state" test was updated to click Confirm
  on the new dialog before asserting the backend call.
Changing the network path (in the Header banner field, via the file
picker, or by typing then blurring) silently dropped the loaded
study with no warning. Loading or applying a different study has
already been gated behind a confirmation dialog for a while, so the
network-path-change path felt jarring by comparison.

Fix:

- ConfirmationDialog: new 'changeNetwork' state with copy
  ("Change Network?" + "The current study will be reloaded from the
  new network file."). pendingNetworkPath added to the dialog
  payload.
- App.tsx:
  * New committedNetworkPathRef tracks the network file the
    currently-loaded study was loaded from. Updated on every
    successful handleLoadConfig / applySettingsImmediate.
  * New requestNetworkPathChange(path): optimistically updates the
    networkPath state (so the input mirrors the typed/picked value),
    then — only if a study is already loaded AND the new path
    differs from the committed one — opens the changeNetwork
    dialog.
  * handleConfirmDialog dispatches the new branch by calling
    handleLoadConfig.
  * handleCancelDialog rolls back the optimistic networkPath update
    on cancel so the Header field re-syncs with the loaded study.
- Header.tsx:
  * New onCommitNetworkPath prop (separate from setNetworkPath so
    typing remains free).
  * The file picker now passes onCommitNetworkPath as its setter,
    so picking a new file routes through the dialog.
  * The input has an onBlur handler that also routes through
    onCommitNetworkPath, covering the manual-typing case.
  * Added data-testid="header-network-path-input" for tests.

Tests:

- ConfirmationDialog.test.tsx: existing "applySettings" test was
  already covering the dialog plumbing; the new "changeNetwork"
  type is exercised end-to-end through App.session.test.tsx.
- Header.test.tsx (2 new):
  * picker routes through onCommitNetworkPath (not setNetworkPath
    directly).
  * blurring the input after editing calls onCommitNetworkPath
    with the new value (uses a stateful harness because Header
    is a controlled component).
- App.session.test.tsx — new "Change Network Path Confirmation"
  describe block (5 new):
  * does not prompt when no study has been loaded yet
  * shows the dialog when typing a different path after load,
    and updateConfig is NOT called yet
  * Confirm: dialog gone, updateConfig called with the NEW path,
    branches re-fetched
  * Cancel: dialog gone, no backend call, Header input reverted
    to the committed path
  * blurring with no actual change does not prompt
Previously the Apply Settings confirmation dialog only fired when
hasAnalysisState() was true (some analysis result, manual sim, or
selection existed). Loading a network and then changing the config
file path in Settings → Paths and clicking Apply silently dropped
the loaded grid with no warning, because no analysis had been run
yet.

Fix:

- App.tsx handleApplySettingsClick now also checks
  committedNetworkPathRef.current — if a study has been loaded at
  all (i.e. the user clicked Load Study or Apply at least once),
  Apply routes through the confirmation pipeline, even when no
  analysis state exists yet. Apply on a brand-new session (nothing
  loaded) still applies immediately.

Tests:

- App.session.test.tsx: extracted an `applyAndConfirm()` helper
  inside "Full State Reset on Apply Settings" and used it from the
  five tests that previously called Apply directly after a Load
  Study (they now correctly walk through the confirmation
  dialog).
- Renamed the "applies settings directly when no analysis state
  exists" test to "applies settings directly when no study has
  been loaded yet" and updated it to render a fresh App without
  calling renderAndLoadStudy(), so it actually exercises the
  truly-empty-session path.
- New regression test "shows confirmation dialog when applying
  settings with a loaded network but no analysis": loads the
  study, opens settings, types a new Config File Path, clicks
  Apply, asserts the dialog appears AND that updateConfig has not
  been called yet — the exact bug scenario the user reported.
@marota
marota merged commit f0ffb58 into main Apr 13, 2026
2 checks passed
marota pushed a commit that referenced this pull request Apr 14, 2026
Session reload no longer loses data introduced by PRs #73/#78/#83/#88:

- handleRestoreSession now restores lines_overloaded_after,
  load_shedding_details, curtailment_details and pst_details on each
  ActionDetail. Previously these were dropped on reload, so the PST /
  load-shedding / curtailment editor cards rendered empty and the
  Remedial Action tab lost its post-action overload halos until the
  user re-ran analysis.
- buildSessionResult persists the sticky-header rho arrays
  (n_overloads_rho / n1_overloads_rho) alongside the overload name
  lists, guarded on matching length so misaligned legacy data is
  omitted instead of saved.
- committedNetworkPathRef is now updated on session restore so the
  "Change Network?" confirmation dialog no longer misfires (or
  silently drops the study) after a reload.

Interaction logging now captures every user gesture the replay
contract needs to faithfully reproduce a session:

- config_loaded and settings_applied include the full settings
  payload (all paths, every recommender threshold including
  min_load_shedding and min_renewable_curtailment_actions, monitoring,
  pre-existing overload threshold, ignore_reconnections,
  pypowsybl_fast_mode).
- settings_tab_changed emits { from_tab, to_tab } and skips no-op
  clicks on the already-active tab.
- New event types action_mw_resimulated and pst_tap_resimulated are
  logged from ActionFeed.handleResimulate / handleResimulateTap with
  the raw user-entered target_mw / target_tap. useActions no longer
  logs manual_action_simulated from handleActionResimulated, which
  conflated the two flows and made replay impossible.

docs/interaction-logging.md is rewritten to reflect all of the above:
documented tab_detached / tab_reattached / tab_tied / tab_untied
visualisation events (previously in types.ts but undocumented),
corrected the details shape for view_mode_changed / asset_clicked /
inspect_query_changed / sld_overlay_* / session_* to match the actual
emitted payloads, documented the applySettings / loadStudy /
changeNetwork cases on contingency_confirmed, and added a new
"Session reload fidelity" section listing exactly which fields are
persisted / restored and which are intentionally ephemeral.

Tests: 695 frontend tests still pass; sessionUtils.test.ts gains
coverage for rho persistence guards and useActions.test.ts now
asserts that handleActionResimulated does not log from the hook.

https://claude.ai/code/session_013qJjLFQWMR91ZfPTRCLFiu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants