⚡ [performance improvement N+1 Query in database iteration] by akutuva21 · Pull Request #109 · akutuva21/PyBioNetGen

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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

akutuva21 · 2026-04-13T13:54:07Z

💡 What:
Replaced an N+1 looping query pattern inside NamingDatabase.findOverlappingNamespace with a batched query implementation. Instead of establishing a new connection and executing a SELECT statement for each file individually, it now chunks the fileList (size 500) and queries the DB using an IN (...) clause. The script builds a mapped dictionary mapping filenames to their rows and processes them locally.

🎯 Why:
The original implementation had a classic N+1 query issue. For N files, it initialized N separate database connections and ran N queries. This incurred heavy I/O overhead and was extremely inefficient, particularly for processing large databases.

📊 Measured Improvement:
On a dataset of 1000 simulated files:

Baseline (N+1 queries): ~0.417s
Optimized (Batched queries): ~0.010s
Speedup: ~40x

PR created automatically by Jules for task 1138012978852035096 started by @akutuva21

Replaces repeated N+1 DB calls inside a loop with a single chunked batched SQL IN query fetching all required records once, speeding up execution from O(N) queries to O(1) batched queries. Co-authored-by: akutuva21 <44119804+akutuva21@users.noreply.github.com>

google-labs-jules · 2026-04-13T13:54:09Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

akutuva21 · 2026-04-13T14:28:24Z

Superseded by #108 namingDatabase batching optimization.

google-labs-jules · 2026-04-13T14:29:20Z

Superseded by #108 namingDatabase batching optimization.

Understood. Acknowledging that this work is now obsolete and stopping work on this task.

…ions Under simulator='bngsim', the routing bridge silently fell back to the legacy BNG2.pl subprocess engine whenever the Python parser (modelapi.bngmodel) could not inspect a BNGL's actions — e.g. a typo'd or unknown simulate argument (atoll, abstol, par_scan_vals) that bngmodel rejects but BNG2.pl tolerates. This violated 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, 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 (deduped across the ~4 routing queries per run). - simulator='subprocess' is unchanged (returns before action inspection). To surface the reason, the routing parse now returns (actions, reason): _parse_bngl_actions_for_routing and the memoized loader (renamed _load_bngl_routing_actions; _load_bngl_actions_for_routing kept as a thin list-only wrapper for its other callers) carry the parse message. The unavailable-BNGsim case already returned ROUTE_ERROR and is untouched. Adds tests covering the bngsim error, the auto one-time-warning fallback, the subprocess no-inspect path, reason propagation through the cache, and an end-to-end repro of the atoll-typo model from the issue.

ActionList.define_parser's list-valued argument grammar was stricter than BNG2.pl/Perl: 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), modelapi.bngmodel raised BNGParseError, so under simulator='bngsim' the bridge couldn't inspect the actions and silently fell back to the legacy subprocess (the #109 class) — the model never ran on bngsim. Real models hit this: RuleHub's Mitra2019/15-igf1r fits and Salazar-Cavazos2019 CHO_EGFR best-fit. 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 = "[" + Optional(delimitedList(quote_word ^ arg_type_expr)) + Optional(",") + "]" Still rejects genuinely-malformed lists (double commas `[1,,2]`, unclosed `[1,2,`). Adds parametrized accept/reject tests over the issue's matrix and the affected real-model forms.

…legacy Follow-up to the #109 contract. 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 BNG2.pl stack (ROUTE_SUBPROCESS was never promoted to ROUTE_ERROR). #109 only closed the parse-failure (actions==None) hole; this closes the unrecognized-action hole. 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 and bngsim alike); auto keeps the legacy fallback for unrecognized actions too. Threads `simulator` into _classify_bngl_actions_for_bngsim (default 'auto', back-compatible) and promotes only the unrecognized-action branch. Adds tests: bngsim unknown-action -> error, auto unknown-action -> subprocess, and bngsim PLA / unsupported-method -> subprocess.

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'. Same silent-downgrade class as #109. Make the classifier RuleMonkey-aware (thread bngsim_has_rulemonkey through classify_bngsim_route -> _classify_bngl_actions_for_bngsim -> _method_supported_by_bngsim_for_routing) so the decision is made up front where strict mode is enforced, not in the simulator-blind runner. Treat nf-without-NFsim and rm-without-RuleMonkey uniformly as *build* incapabilities (_BNGSIM_OPTIONAL_COMPONENT_METHODS): BNGsim could run them with the component installed, so simulator='bngsim' errors (surfacing "install NFsim/RuleMonkey") while auto falls back to legacy. This matches the existing direct BNG-XML nf behavior (classify already errored there under bngsim). NOTE: this revises the BNGL nf-without-NFsim case shipped in the previous commit (was subprocess in both modes) so nf and rm are consistent and the strict-bngsim silent gap is closed for both. pla stays subprocess in both modes (a categorical incapability, not a missing component); the cvode alias keeps its legacy route. Updates tests accordingly and adds rm coverage.

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