Fix BNGsim bridge silent fallbacks: strict routing error (#109) + Perl-faithful list-arg grammar (#110)

[wshlavacek]

Summary

Two related fixes to the optional BNGsim bridge, both addressing cases where a model silently ran on the legacy BNG2.pl subprocess when the caller expected BNGsim. One hardens the routing contract; the other removes the most common trigger.

Closes #109
Closes #110

#109 — simulator='bngsim' must error, not silently fall back

Under simulator='bngsim', the routing bridge silently routed to the legacy BNG2.pl subprocess whenever the Python parser (modelapi.bngmodel) couldn't inspect a model's actions — e.g. an unrecognized simulate argument that bngmodel rejects but BNG2.pl tolerates. That violates the simulator='bngsim' contract ("use BNGsim, or error"): the caller got legacy output with no exception and no warning. A parity sweep over 895 models hit this on 9 models.

_classify_bngl_actions_for_bngsim() returned ROUTE_SUBPROCESS for the un-inspectable case unconditionally, with no reference to simulator.

Fix (bionetgen/core/tools/bngsim_bridge.py), at the classify_bngsim_route call site where simulator is in scope:

• simulator='bngsim' + un-inspectable actions → ROUTE_ERROR, propagating the underlying parse reason so the caller learns why it couldn't route to BNGsim. Both consumers (main.py, runner.run) already raise BNGSimError(route.reason) on ROUTE_ERROR.
• simulator='auto' keeps the subprocess fallback but emits a one-time WARNING per file.
• simulator='subprocess' is unchanged (returns before action inspection).

To surface the reason, the routing parse now returns (actions, reason) through the memoized loader (_load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers). The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched.

#110 — action grammar: accept e-notation and trailing commas in list args

ActionList.define_parser (bionetgen/core/utils/utils.py) was stricter than BNG2.pl/Perl for list-valued arguments. arg_type_list matched elements with pp.Word(pp.nums + ".") (digits and . only) inside a plain pp.delimitedList, so it rejected:

• scientific notation, e.g. par_scan_vals=>[2.3e-10,5.1e-10]
• a trailing comma, e.g. par_scan_vals=>[1,2,3,]

Both are valid Perl that BNG2.pl parses and runs. When a model used either (commonly par_scan_vals on parameter_scan), bngmodel raised BNGParseError — which is exactly what triggered the #109 silent-fallback class. Real models hit this: RuleHub's Mitra2019/15-igf1r fits (e-notation and trailing commas) and Salazar-Cavazos2019 CHO_EGFR best-fit (e-notation).

Fix: broaden arg_type_list to use arg_type_expr (which already spans e/E and +/-), allow an empty list, and tolerate one optional trailing comma:

arg_type_list = (
    "[" + pp.Optional(pp.delimitedList(quote_word ^ arg_type_expr)) + pp.Optional(",") + "]"
)

Still rejects genuinely-malformed lists — double commas ([1,,2]) and unclosed lists ([1,2,).

#109 follow-up — strict mode also errors on unrecognized actions

#109's first fix closed the parse-failure hole (actions can't be inspected → None). But classify_bngsim_route returned the _classify_bngl_actions_for_bngsim result verbatim, so under an explicit simulator='bngsim' request a BNGL with an action the router doesn't recognize still silently ran on the legacy stack (its ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR).

BNGsim can run such a model via the BNG2.pl backend hook — the router just can't confirm an unknown action is safe to delegate — so strict mode now surfaces ROUTE_ERROR with the reason instead of degrading silently. PLA and BNGsim-unsupported methods are genuine BNGsim incapabilities, so they stay on the subprocess route in every mode; auto keeps the legacy fallback for unrecognized actions too. simulator is threaded into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible).

#109 follow-up — unknown simulation methods error instead of silent legacy

The router treated every BNGsim-unsupported method the same (ROUTE_SUBPROCESS), so a malformed/unknown method — a typo like oed, or quadratic — silently ran on the legacy stack in both auto and bngsim. BNG2.pl's own simulate() validates the method against its $METHODS hash (cvode/ssa/pla/psa/nf, plus the ode alias) and rejects anything else ("Simulation method '...' is not a valid option"), so this is faithful, not stricter than BNG2.pl.

Now split (_unsupported_method_route):

• A valid BNG method this BNGsim build can't run — pla, or nf without an NFsim build — stays on the subprocess route (genuine/build incapability), in both modes.
• A method outside the valid set errors in both modes, surfacing it.

_KNOWN_BNGL_METHODS = {ode, cvode, ssa, psa, pla, nf, rm} is taken verbatim from BNG2.pl's $METHODS keys (+ ode alias, + rm RuleMonkey extension), so valid methods like the cvode alias keep their existing legacy routing rather than newly erroring.

Note the deliberate asymmetry with actions: BNG2.pl supports many actions outside the router's curated list (setVolume, setOption, readSBML, quit, …), so auto keeps the legacy fallback for unrecognized actions (only bngsim errors). Methods are a closed, BNG2.pl-validated set; actions are open.

#109 follow-up — rm/RuleMonkey awareness; build incapabilities error under strict mode

method=>"rm" routes to BNGsim's RuleMonkey (via the rm→nf rewrite + backend hook) only when RuleMonkey is installed — but the router marked rm supported unconditionally (_method_supported_by_bngsim_for_routing never checked RuleMonkey, unlike nf/NFsim), and classify_bngsim_route took no RuleMonkey arg. So an rm model on a BNGsim without RuleMonkey still classified as ROUTE_BNGL_BNGSIM, then run_bngl_with_bngsim (which hardcodes simulator="auto") silently fell back to legacy — invisibly even under simulator='bngsim'.

Now the classifier is RuleMonkey-aware (bngsim_has_rulemonkey threaded through), so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. nf-without-NFsim and rm-without-RuleMonkey are treated uniformly as build incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey"), auto falls back. This matches the existing direct BNG-XML nf behavior. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route.

Why together

#110 makes these models parseable; #109 ensures that when something still isn't inspectable, bngsim errors instead of silently degrading. Shipping them together closes the silent-fallback gap end to end.

Tests

• tests/test_bngsim_routing_classifier.py: bngsim error path, auto one-time-warning fallback, subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the unrecognized-argument model.
• tests/test_bng_parsing.py: parametrized accept/reject grammar tests over the issue Action grammar rejects scientific-notation and trailing commas in list args (par_scan_vals) — stricter than BNG2.pl, causes silent legacy fallback under simulator='bngsim' #110 matrix and the affected real-model forms.
• Targeted regression sweep over parsing/action/scan/routing test files passes.