Skip to content

Split bngsim_model.py (2.7k-line simulator-interface god-file) into a navigable package (CQ-1b) #408

@wshlavacek

Description

@wshlavacek

What this is

pybnf/bngsim_model.py is a 2,749-line god-file — the BNGsim simulator-interface layer, the most bngsim-coupled module in the package. M1 split algorithms.py into a navigable package but the refactor guide explicitly scoped this file out; it has no plan yet (PUNCHLIST item CQ-1b). This issue is that plan's starting point.

It holds two model classes plus ~80 module-level helpers:

  • BngsimModel(NetModel)bngsim_model.py:983 (the ODE/SSA path: action execution, protocol runs, parameter scans)
  • BngsimNfModel(Model)bngsim_model.py:2219 (the NFsim path)
  • dataclasses _SimulateActionState, _SimulateRunPlan, _ScanSettings

Cohesive sub-domains (natural split axes)

The helpers already cluster into clear, mostly-independent groups:

  1. BNGL action-line parsing_collapse_action_line_continuations, _extract_action_body, _split_top_level_commas, _parse_action_value, _parse_action_dict, _parse_{simulate,parameter_scan,bifurcate,set_parameter,set_concentration,add_concentration,…}_action, the _is_reset/save_* predicates. Pure text → dict, simulator-free, highly testable.
  2. Expression / parameter evaluation_build_safe_eval_namespace, _eval_numeric, _eval_model_expression, _parse_bngl_param_block, _evaluate_bngl_params, _parse_net_species_initializers, _model_param_values, _build_mutant_param_set. Also pure (this is the ROB-5 rint/safe-eval home).
  3. Backend classification & NFsim session management_classify_action_method_backend, _allowed_bngsim_backends_for_action, classify_actions_for_bngsim, actions_compatible_with_bngsim, _normalize_*_method, _nf_session_backend_*, _get/create/destroy_nf_session, missing_bngsim_nf_action_support.
  4. Scan resolution & execution_ScanSettings, _resolve_scan_points, _resolve_sample_times, the _scan_* methods (newton/parity/protocol/continuation/independent steady-state), _assemble_scan_data.
  5. The two model classes (BngsimModel, BngsimNfModel) + the simulate dataclasses.

Groups 1 and 2 are pure and CI-runnable (no bngsim wheel needed) — the highest-value, lowest-risk first peels, and extracting them lets us add CI-tier unit tests that don't exist today.

Why it needs its own plan + a safety-net audit first

  • bngsim is a private, mac-only wheel and is not in CI. The hosted tests/integration tiers run bngsim-less (PYBNF_NO_BNGSIM=1, conftest auto-skips). So the e2e BngsimModel.execute / scan / NFsim paths (groups 3–5) are only exercised locally on a mac. The split must (a) never add a hard top-level bngsim import to a relocated module (would red the CI tier), and (b) treat the local bngsim e2e suite as the behavioral contract for the simulator-coupled parts.
  • Facade/patch seams (ADR-0001). test_bngsim_model.py / test_bngsim_bridge.py patch many names by where they're imported. Moving a name breaks any test that patches it on the old module. Audit every patch(...) / monkeypatch target before moving code and repoint to where it now lives (the recurring M1 lesson).

Proposed approach (M1-style, behavior-preserving)

  1. Safety-net audit — enumerate the bngsim test suite, the facade/patch targets, and which paths are CI-runnable vs mac-only. Write it down before touching code.
  2. Skeleton, no-oppybnf/bngsim/ package (or keep bngsim_model.py as a thin re-export facade), suite green.
  3. Peel the pure groups first (parsing → expressions), one cohesive group per commit, byte-identical relocation, and add CI-tier unit tests for the now-importable pure helpers.
  4. Then the bngsim-coupled groups (sessions, scans, model classes) under the local bngsim e2e net.
  5. Full net green after each commit: uvx ruff check . + fast + slow, plus the local bngsim suite for the coupled moves (run sequentially — resource-limited machine).

Acceptance

  • bngsim_model.py is a navigable package; no single file is a 2.7k-line god-file.
  • The pure parsing/expression helpers have CI-runnable unit tests (coverage that doesn't exist today).
  • bngsim-less CI tier stays green; local bngsim e2e suite stays green; no behavior change.

Refs: PUNCHLIST CQ-1b (dev/PUNCHLIST.md), dev/refactor-guide.md (scopes this file out), ADR-0001 (patch-where-defined seam rule).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions