Skip to content

Refactor: Extract stateless helpers from mixins into focused modules - #106

Merged
marota merged 10 commits into
claude/add-action-overview-filters-OE36Zfrom
claude/merge-main-update-docs-n5JrT
Apr 22, 2026
Merged

marota merged 10 commits into
claude/add-action-overview-filters-OE36Zfrom
claude/merge-main-update-docs-n5JrT

Conversation

@marota

@marota marota commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR decomposes three large service mixins (simulation_mixin, analysis_mixin, diagram_mixin) by extracting their internal logic into focused, stateless helper modules. The public API remains unchanged; orchestrator methods now delegate to pure functions that take their dependencies as arguments.

Key Changes

Backend Refactoring

Simulation helpers (expert_backend/services/simulation_helpers.py)

  • Extracted 468 lines of pure functions from simulate_manual_action and compute_superposition
  • Covers action canonicalization, topology extraction, PST tap clamping, rho computation, and result serialization
  • Enables unit testing without service instantiation (new test file: test_simulation_helpers.py)

Analysis helpers (expert_backend/services/analysis/)

  • Split into four focused modules:
    • action_enrichment.py — load shedding, curtailment, PST, and topology details
    • mw_start_scoring.py — MW-at-start computation per action type
    • analysis_runner.py — AC→DC fallback wrapper
    • pdf_watcher.py — overflow PDF discovery
  • Removed ~600 lines of instance methods from AnalysisMixin; replaced with thin orchestrators that inject dependencies
  • New test file: test_analysis_helpers.py (510 lines)

Diagram helpers (expert_backend/services/diagram/)

  • Split into seven modules:
    • layout_cache.py — grid layout JSON loader with mtime-based caching
    • nad_params.py — default NadParameters factory
    • nad_render.py — diagram generation + NaN stripping
    • sld_render.py — SLD SVG + metadata extraction
    • overloads.py — overload filtering and current scans
    • flows.py — branch and asset flow extractors
    • deltas.py — terminal-aware delta math (pure numerics)
  • New test file: test_diagram_helpers.py (427 lines)

Frontend Refactoring

SVG utilities (frontend/src/utils/svg/)

  • Extracted from monolithic svgUtils.ts (2028 → 60 lines) into:
    • idMap.ts — DOM ID caching
    • svgBoost.ts — large-grid scaling
    • metadataIndex.ts — O(1) metadata lookups
    • highlights.ts — contingency/overload halo rendering
    • actionPinData.ts — pure pin descriptor builders (severity, anchors, labels)
    • actionPinRender.ts — DOM injection for action-overview pins
    • fitRect.ts — bounding-box computation
    • deltaVisuals.ts — delta styling helpers
  • Each module has corresponding unit tests (8 new test files)

Code Quality & CI

  • Added scripts/code_quality_report.py — offline metrics aggregation (LoC, largest files, function lengths, anti-patterns)
  • Added scripts/check_code_quality.py — CI gate that enforces thresholds
  • Added scripts/test_code_quality_report.py — unit tests for the reporter (7 tests)
  • New GitHub Actions workflow: .github/workflows/code-quality.yml
  • Updated CircleCI config with code-quality job
  • Added CONTRIBUTING.md with development setup and code-quality check instructions
  • Added .editorconfig and .env.example for consistency

Documentation

  • Updated CLAUDE.md with new module structure
  • Updated docs/architecture/code-quality-analysis.md with continuous reporting details
  • Updated docs/features/action-overview-diagram.md with new filter UI
  • Updated pyproject.toml with quality extras group

Implementation Details

  • No public API changes: All orchestrator methods retain their original signatures and behavior
  • Dependency injection: Helpers take their dependencies (observations, network_service, pst_tap_info callable)

https://claude.ai/code/session_0176eivSfH1GiV9CPc1iGqMo

claude and others added 10 commits April 20, 2026 16:57
Adds automated metric reporting and a CI gate that locks in the
reductions documented in docs/architecture/code-quality-analysis.md,
and closes remaining housekeeping issues called out in the audit.

Tooling
- scripts/code_quality_report.py: AST-walks backend sources + globs
  frontend for per-file LoC, longest-function report, and smell
  counts (print / traceback.print_exc / silent except / any /
  @ts-ignore / weak casts). JSON + Markdown output.
- scripts/check_code_quality.py: PR gate. Non-zero exit when a
  threshold is exceeded (see CONTRIBUTING.md).
- scripts/test_code_quality_report.py: 7 unit tests on the AST
  smell-walker — strings containing `print(` are not flagged,
  logged except is not silent, etc.
- pyproject.toml: [quality] extra (ruff + radon) and a narrow ruff
  ruleset (F, E9) scoped to real bugs.
- .github/workflows/code-quality.yml + .circleci/config.yml
  code-quality job: run ruff, the gate, the reporter unit tests,
  and upload reports/ as a CI artifact.

Bugs surfaced by the new tooling
- simulation_mixin.py: self-reference called static method via
  RecommenderService.* without importing it (F821 NameError if
  branch ever taken). Switched to self.*.
- network_service.py: get_load_voltage_levels_bulk fell through
  without a loop body. Implemented to mirror get_generator_types_bulk.
- analysis_mixin.py: traceback.print_exc() replaced with
  logger.exception(); stray `except Exception: pass` logged via
  logger.debug; redundant shadowed imports removed.
- main.py: unused GZipMiddleware import + stray print() calls
  replaced with logger.warning; consolidated imports; CORS now
  honours CORS_ALLOWED_ORIGINS env var (with credentials-on-wildcard
  guard).
- 9 auto-fixed f-strings without placeholders via `ruff --fix`.

Frontend
- Dropped unused deps: framer-motion, lucide-react.
- SettingsModal.tsx: empty .catch(() => {}) handlers log via
  console.error.

Housekeeping
- CONTRIBUTING.md, .editorconfig, .env.example added; referenced
  from root CLAUDE.md.
- .gitignore: exclude reports/ and .env.
- Root CLAUDE.md refreshed — dropped stale test_*.py references,
  added scripts/ + code-quality sections.

Docs
- docs/architecture/code-quality-analysis.md updated to 2026-04-20:
  new Metrics Summary from the reporter, Continuous-Quality Tooling
  section (6b), Delta section (9), and each resolved row marked.
…rposition

The two longest functions flagged by the continuous reporter:

  simulate_manual_action:  599 → 146 lines
  compute_superposition:   285 → 108 lines

Both are now thin orchestrators that delegate each phase to a named
helper — the data flow is explicit in argument lists instead of hidden
as closures over locals.

New stateless module: expert_backend/services/simulation_helpers.py

  canonicalize_action_id        parse_pst_tap_id / clamp_tap
  classify_action_content       is_pst_action
  pst_fallback_line_idxs        compute_reduction_setpoint
  build_care_mask               resolve_lines_overloaded
  compute_action_metrics        extract_action_topology
  serialize_action_result       normalise_non_convergence
  build_combined_description    compute_combined_rho

New private methods on SimulationMixin (need self state):

  _inject_action_content_entries / _fetch_n_and_n1_observations
  _create_dynamic_{curtailment,load_shedding,pst} / _create_dynamic_actions_if_needed
  _promote_recent_actions_to_dict / _apply_target_mw_updates / _apply_target_tap_updates
  _build_combined_action_object / _resolve_action_description_and_content
  _register_action_result
  _ensure_pair_simulated / _identify_elements_with_pst_fallback
  _superposition_lines_overloaded / _augment_superposition_result
  _log_dict_action_snapshot / _log_per_line_rho

Coverage
- 66 new unit tests in expert_backend/tests/test_simulation_helpers.py —
  one class per helper, exercising islanding, PST-tap-only topologies,
  heuristic power reduction, pre-existing overload exclusion,
  combined-pair description, sign-preserving rho superposition.
- 130 existing simulation + superposition tests continue to pass
  (same mocks, same call order, same public contract). Pre-existing
  test-pollution failures (19) are unchanged — same before and after
  the refactor.

Docs
- Updated §6 metrics in code-quality-analysis.md with the new
  longest-function table.
- Added §10 delta documenting the decomposition + next candidates
  (run_analysis 169, update_config 166, _enrich_actions 125).
The largest frontend source file, flagged by the continuous quality
reporter. Split into 8 modules under `frontend/src/utils/svg/`, each
with a single responsibility. `svgUtils.ts` is now a 60-line barrel
that re-exports every symbol — every caller keeps working unchanged.

Module              Lines   Responsibility
─────────────────── ─────── ─────────────────────────────────────────
idMap.ts              29    Cached DOM-id map for an SVG container
svgBoost.ts          122    Dynamic font/node scaling + processSvg
metadataIndex.ts      40    Build MetadataIndex from pypowsybl meta
highlights.ts        422    Overloaded / action-target / contingency
                            halos; line & VL target resolution
deltaVisuals.ts      137    Delta flow coloring + text replacement
actionPinData.ts     344    Pin descriptors + severity palette +
                            anchor resolution (pure, no DOM)
actionPinRender.ts   537    DOM injection for pins + highlights +
                            click semantics + scale math
fitRect.ts           125    Padded viewBox computation

Largest frontend file is now App.tsx at 1370 lines — the state
orchestration hub, exempt from the component-size ceiling by design.

New helpers promoted to the public API (formerly inline closures):

- formatPinLabel(details)       — percentage / DIV / ISL / em-dash
- formatPinTitle(idLabel, det)  — hover tooltip string
- fanOutColocatedPins(pins)     — circular fan-out of colocated pins
- curveMidpoint(p1, p2)         — quadratic Bezier midpoint + ctrl
- computePinScale(...)          — pin scale math for rescaler
- rectFromBounds (internal)     — shared MIN_SPAN + padding rules

Coverage
- 61 new unit tests across 5 co-located test files (idMap,
  svgBoost, metadataIndex, actionPinData, actionPinRender,
  fitRect).
- Existing svgUtils.test.ts (144 tests) unchanged and all green.
- Full frontend suite: 1000 tests passing.
- TypeScript strict build clean; ESLint zero warnings.

Docs
- Added §11 Delta in docs/architecture/code-quality-analysis.md
  with the module table and test coverage list.
After the svgUtils.ts → utils/svg/* decomposition (a26ce0e), the
following three Layer 4 invariant regexes no longer matched their
scoped file_hint (the barrel file) and reported FAIL:

  - pin_severity_uses_monitoringFactor    (computeActionSeverity)
  - combined_pairs_filter_estimated       (buildCombinedActionPins)
  - pin_resolver_is_topology_first        (resolveActionAnchor)

All three contracts live on pure pin descriptors / severity / anchor
resolution and now have a focused home in
`frontend/src/utils/svg/actionPinData.ts`. Update each file_hint to
point there — the regex bodies stay unchanged.

Verified: `python scripts/check_invariants.py` reports 10/10
satisfied.
Adds a step that pipes `reports/code-quality.md` into
`$GITHUB_STEP_SUMMARY` so the metrics show up directly on the
workflow run page under the "Summary" tab — no artifact download
needed. `if: always()` ensures the summary is published even when
earlier steps (ruff, gate) fail, so a regression still surfaces
the full metrics view.

The upload-artifact step stays for machine-readable consumption
(code-quality.json) and 30-day history.
analysis_mixin.py was the largest backend file (1,116 lines). Split
into four focused modules under a new `services/analysis/` package:

  pdf_watcher.py         43    Overflow PDF glob + mtime search
  action_enrichment.py   389   LS / curtailment / PST details, rho
                               scaling, topology extraction,
                               non-convergence normalisation — pure
  mw_start_scoring.py    341   MW-at-start dispatcher + per-type math
                               (LS, curtailment, line disco, PST,
                               open coupling) — pure
  analysis_runner.py     193   Legacy AC→DC worker + PDF-polling
                               generator + derive_analysis_message

analysis_mixin.py shrank 1,116 → 509 lines. The three public entry
points (run_analysis_step1 / run_analysis_step2 / run_analysis) are
now thin orchestrators + `_enrich_actions` /
`_compute_mw_start_for_scores` iterators that delegate to the
stateless helpers.

Dependency injection preserves @patch compatibility

Two library functions that tests traditionally @patch
(`get_virtual_line_flow`, `run_analysis`) are now threaded into the
helpers as optional callables. The mixin reads the module-level
name at call time and forwards it — so existing
`@patch('expert_backend.services.analysis_mixin.*')` tests keep
working unchanged.

Legacy instance-method wrappers kept
- `_compute_load_shedding_details`, `_compute_curtailment_details`,
  `_compute_pst_details`, `_is_renewable_gen`
- `_mw_start_load_shedding`, `_mw_start_curtailment`,
  `_mw_start_open_coupling`, `_get_action_mw_start`,
  `_get_pst_tap_start`

They now delegate to the module-level helpers. Tests that patched
these methods on the instance (test_recommender_regressions) keep
passing without modification.

Coverage
- 68 new unit tests in tests/test_analysis_helpers.py covering
  every helper in isolation (pdf_watcher mtime filter, non-convergence
  normalisation, lines-overloaded reconstruction, load-shedding /
  curtailment / PST details, MW-start dispatcher, analysis-message
  derivation, …).
- Diffed full-suite failure set against pre-refactor baseline —
  identical 19 pre-existing test-pollution failures on both sides,
  zero new regressions.
- Quality gate green, ruff (E9/F) clean.

Top-5 longest functions after the pass
  update_config             166
  simulate_manual_action    146
  compute_superposition     112
  get_action_variant_sld     97
  _compute_deltas            92

`run_analysis` (was 169 lines) and `_enrich_actions` (was 125 lines)
both fell out of the top-5.

Docs
- Added §12 Delta to docs/architecture/code-quality-analysis.md
  with the module table, DI strategy, and next candidates.
diagram_mixin.py was the second-largest backend file after the
analysis sweep. Split into seven focused modules under a new
services/diagram/ package:

  layout_cache.py    63   (path, mtime)-keyed grid_layout.json loader
  nad_params.py      41   Default NadParameters factory (perf-tuned)
  nad_render.py      89   generate_diagram + NaN element stripping
  sld_render.py      36   SLD SVG + metadata extraction with fallbacks
  overloads.py      131   Overload filtering + per-element currents
  flows.py           67   Branch + asset flow extractors (vectorised)
  deltas.py         241   Terminal-aware flow-delta math (pure)

diagram_mixin.py shrank 974 → 469 lines. Every public method
(get_network_diagram, get_n1_diagram, get_action_variant_diagram,
get_n_sld, get_n1_sld, get_action_variant_sld) is a short
orchestrator that switches the right variant, calls the stateless
helpers, and stashes results.

Five new private helpers factor out patterns that previously
repeated across methods:

  _require_action                  validate + fetch the action entry
  _lf_status_for_variant           load-flow status with variant cache
  _snapshot_n1_state               N-1 flows + assets with variant restore
  _attach_flow_deltas_vs_base      populate flow/asset deltas on diagram
  _attach_convergence_from_obs     copy lf_converged from observation
  _diff_switches                   switch-state diff between variants

Subtle bug fixed by the refactor

test_sld_highlight.py uncovered an ordering dependency on the SLD
manual-action path: `changed_switches` must be captured BEFORE
attempting flow extraction so that mock/malformed networks with
missing flows still return the switch diff. The new orchestrator
splits the try/except into separate blocks — one for the switch
diff, one for the delta math — so each can fail independently.

Coverage
- 39 new unit tests in tests/test_diagram_helpers.py covering every
  helper in isolation (layout cache eviction, NaN stripping,
  overload filtering with N-state exclusion, terminal-aware delta
  math with direction-flip, vectorised equivalent against scalar
  reference, asset-delta categorisation, …).
- Full-suite diff against pre-refactor baseline: identical 19
  pre-existing test-pollution failures, zero new regressions.
- Quality gate green, ruff (E9/F) clean.

Top-5 longest functions after the pass
  update_config                 166
  simulate_manual_action        146
  compute_superposition         112
  compute_action_metrics         87
  _augment_superposition_result  81

Every diagram-related function fell out of the top-5.

Docs
- Added §13 Delta to docs/architecture/code-quality-analysis.md.
Code-quality gate + decomposition sweep (5 modules)
Reconciles the action-overview filter / un-simulated-pin feature branch
with main's parallel svgUtils.ts → `./svg/*` decomposition.

The only conflict was `frontend/src/utils/svgUtils.ts` (both sides
rewrote it). Resolution: accept main's barrel file and port the
branch's additions into the decomposed modules:

- `svg/actionPinData.ts`: add optional `unsimulated` and
  `dimmedByFilter` fields on `ActionPinInfo`; export
  `actionPassesOverviewFilter`; extend `buildActionOverviewPins` with
  an optional `overviewFilter` param; add `buildUnsimulatedActionPins`
  + its internal tooltip helper.
- `svg/actionPinRender.ts`: extend `ApplyPinsOptions` with
  `unsimulatedPins` + `onUnsimulatedPinDoubleClick`; teach
  `resolvePinFill` / `renderUnitaryPin` about `dimmedByFilter`
  (washed-out fill, 0.4 opacity, `data-dimmed-by-filter` attr);
  add `renderUnsimulatedPin` for dashed grey previews;
  wire them into `applyActionOverviewPins`.
- `svgUtils.ts` barrel: re-export `actionPassesOverviewFilter` and
  `buildUnsimulatedActionPins`.

All 1079 frontend tests pass; typecheck + lint clean.

https://claude.ai/code/session_0176eivSfH1GiV9CPc1iGqMo
Extends the action-overview doc with the features landed in PR #105:

- Overview paragraph + ASCII diagram updated to show the consolidated
  single-row header (counter + category chips + All/None + threshold
  slider + Show-unsimulated + action-type chips) and the un-simulated
  pin glyph.
- New `Un-simulated action pin` sub-section under Pin anatomy: SVG
  structure, anchor fallback via minimal stub, grey palette, dashed
  stroke, single/double-click semantics, tooltip enrichment contract.
- New `Filter-dimmed constituent pin` sub-section: `dimmedByFilter`
  flag, `data-dimmed-by-filter` attribute, 0.4 opacity rationale.
- New top-level `Filtering` section: ActionOverviewFilters shape and
  defaults, severity categories, threshold slider semantics
  (including the null-max_rho bypass), action-type chip classifier
  rules (coupling signal, commits f356c2e + d479516 regressions),
  three-pass protected-constituent algorithm, shared predicate
  contract with ActionFeed, interaction-logging events.
- Auto-fit dependency list updated to include un-simulated pins.
- New interaction entry: double-click on un-simulated pin → manual
  simulation.
- Files table refreshed: svgUtils.ts is now a barrel, new rows for
  actionPinData.ts, actionPinRender.ts, ActionTypeFilterChips.tsx,
  actionTypes.ts.
- Test-coverage table refreshed with the new filter / un-simulated /
  action-type / protected-constituent / re-colour regression cases.
- Performance notes: filter memo granularity + single-append
  ordering for un-simulated render.

https://claude.ai/code/session_0176eivSfH1GiV9CPc1iGqMo
@marota
marota merged commit c98e741 into claude/add-action-overview-filters-OE36Z Apr 22, 2026
marota pushed a commit that referenced this pull request Apr 22, 2026
… dynamic fix

Captures the post-0.6.0 work: svgPatch DOM-recycling (PR #108),
Action Overview filters + unsimulated pins (PR #105, #107),
code-quality gate + 5 decomposition passes (PR #104, #106),
docs reorganisation (PR #103), App.tsx hook extraction
(PR #109), and the dynamic reco_ reconnection-action fix on
the current branch.

https://claude.ai/code/session_01Tzp2fdUas3Y9vNZy6dxxuC
marota pushed a commit that referenced this pull request Apr 30, 2026
Reconciliation of section 2 (0.5.0):
- Drop misattributed PRs that were actually pre-rebrand: save/reload
  (#49/#52), MW Start (#62), interaction-logging (#64), SLD highlights
  (#63), load shedding initial integration (#61). All now properly
  cited in section 1.5–1.6.
- Disambiguate App.tsx refactor history: PR #56 (hooks, 2100 → 800,
  pre-rebrand) vs PR #74 (components, 1000 → 650, 0.5.0) vs PR #75
  (memoization Phase 2, same LoC).
- Add accurate 0.5.0 PRs: #66 (vectorization w/ benchmark table),
  #69/#70/#71 (UI polish), #72 (curtailment), #73 (loads_p/gens_p
  format + configurable MW), #74/#75 (App.tsx decomposition),
  #78 (PST tap re-simulation), #84/#86/#87/#90 (detachable tabs).
- Add a recap table summarizing what's truly new in 0.5.0.

Diagrams added (Mermaid, GitHub-rendered):
- Gantt timeline of all 4 phases (top of doc).
- High-level architecture (frontend / backend / data).
- Two-step analysis sequence diagram (section 1.6).
- App.tsx LoC evolution flow (section 2.4).
- Backend mixin decomposition before/after PR #104/#106 (section 3).
- PyPSA-EUR pipeline flowchart (section 4).

https://claude.ai/code/session_01Pg7fuCUG2edfm5PyHS6SbN
marota pushed a commit that referenced this pull request May 17, 2026
``diagram_mixin.py`` hit 1220 lines after the prewarm method was
added inline, tripping the 1200-line ceiling enforced by
``scripts/check_code_quality.py``.

Move the heavy lifting into a focused stateless helper
(``services/diagram/obs_prewarm.py:build_prewarmed_obs``) following
the PR #104 / PR #106 mixin → helper-package convention: the helper
takes its dependencies (cached env context, simulation-env factory)
as keyword arguments and returns the (obs, variant_id, elements)
tuple; the mixin keeps the writes to ``_cached_obs_n1*`` so the
cache invariants stay in one place.

``DiagramMixin._cache_obs_for_variant`` becomes a thin wrapper.
Behaviour is unchanged — the existing
``test_obs_prewarm_for_step1.py`` suite (cache hit / miss /
fallback / DO_RECO_MAINTENANCE gate / signature-introspection
fallback) still passes.

File size: 1220 → 1198 (under the ceiling).

Updates:

* ``expert_backend/services/diagram/obs_prewarm.py`` — new module.
* ``expert_backend/services/diagram_mixin.py`` — slim wrapper + import.
* ``expert_backend/CLAUDE.md`` + root ``CLAUDE.md`` — mention the new
  helper in the diagram-package index.
* ``docs/backend/recommender_models.md`` § Execution-time breakdown —
  reference the helper module.
marota pushed a commit that referenced this pull request May 17, 2026
…odule

``diagram_mixin.py`` had drifted near the 1200-line ceiling for the
third time in this branch (1198/1200 after the previous prewarm
extraction). The single biggest contributor was
``get_action_variant_diagram_patch`` at 279 lines — by far the heaviest
method in the file, plus three patch-specific helpers totalling ~200
more lines.

Move the whole action-patch pipeline into a focused module
``services/diagram/action_patch.py`` (following the PR #104 /
PR #106 mixin → helper-package convention):

* ``compute_vl_topology_diff`` — bus-count diff per VL.
* ``get_disconnected_branches_from_snapshot`` — branch state from
  pre-captured connectivity frames.
* ``extract_vl_subtrees_with_edges`` — per-VL focused-NAD subtrees
  for the SVG splice. Takes ``generate_diagram`` as a callable so the
  NAD-generation seam stays the mixin's.
* ``build_action_patch_payload(service, action_id)`` — public entry
  point the mixin wraps in two lines. Reads the dozen-odd service
  collaborators it needs through the passed-in service handle.

Three private orchestrator helpers keep the entry point under the
function-LoC ceiling: ``_extract_convergence_status``,
``_capture_action_snapshots`` (fold the five try/except snapshot
blocks into one routine), ``_unpatchable_response`` (the standard
fallback payload).

Test compatibility: ``tests/test_diagram_patch_helpers.py`` calls
``DiagramMixin._compute_vl_topology_diff`` and
``DiagramMixin._get_disconnected_branches_from_snapshot`` as static
methods — both are re-exported as thin static-method wrappers on the
mixin so the tests keep working unchanged (all 22 pass).

File sizes:
* ``diagram_mixin.py``: 1198 → 769 lines (-429, 36% reduction).
* ``services/diagram/action_patch.py``: new, 531 lines.

Comfortable 431-line buffer below the 1200 ceiling means we can land
several rounds of new diagram features before hitting the wall again.
Per the user's request, this is the targeted "option 1" extraction
discussed in the earlier review.

CLAUDE.md indexes (root + ``expert_backend/``) updated to mention the
new module.
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