Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ gdpr_consent/node_modules/
*~
.streamlit/secrets.toml
docs/superpowers/
.venv/
28 changes: 25 additions & 3 deletions src/workflow/CommandExecutor.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import threading
from pathlib import Path
from .Logger import Logger
from .ParameterManager import ParameterManager
from .ParameterManager import ParameterManager, bool_param_paths_from_param_xml_ini
import sys
import importlib.util
import json
Expand Down Expand Up @@ -216,7 +216,7 @@ def read_stderr():
stdout_thread.join()
stderr_thread.join()

def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> bool:
def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}, tool_instance_name: str = None) -> bool:
"""
Constructs and executes commands for the specified tool OpenMS TOPP tool based on the given
input and output configurations. Ensures that all input/output file lists
Expand All @@ -234,6 +234,10 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b
tool (str): The executable name or path of the tool.
input_output (dict): A dictionary specifying the input/output parameter names (as key) and their corresponding file paths (as value).
custom_params (dict): A dictionary of custom parameters to pass to the tool.
tool_instance_name (str, optional): Key for ``params.json`` when it differs
from ``tool`` (e.g. multiple instances). Defaults to ``tool``.
Custom parameters whose keys appear in the tool's ParamXML ``type="bool"``
entries are passed as valueless CLI flags (``-name`` only when enabled).

Returns:
bool: True if all commands succeeded, False if any failed.
Expand Down Expand Up @@ -261,8 +265,15 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b

commands = []

# Load parameters for non-defaults
params = self.parameter_manager.get_parameters_from_json()

topp_tool_ini_path = Path(self.parameter_manager.ini_dir, f"{tool}.ini")
# Keys of type="bool" in the .ini: TOPP treats these as on/off flags (omit value when off)
topp_bool_flag_param_keys = (
bool_param_paths_from_param_xml_ini(topp_tool_ini_path, tool)
if topp_tool_ini_path.exists()
else set()
)
# Construct commands for each process
for i in range(n_processes):
command = [tool]
Expand All @@ -284,6 +295,16 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b
# Add non-default TOPP tool parameters
if tool in params.keys():
for k, v in params[tool].items():

if k in topp_bool_flag_param_keys and v != "":
# CLI flag: include "-k" only when enabled
if isinstance(v, str):
is_enabled = v.lower() == "true"
else:
is_enabled = bool(v)
if is_enabled:
command += [f"-{k}"]
continue
command += [f"-{k}"]
# Skip only empty strings (pass flag with no value)
# Note: 0 and 0.0 are valid values, so use explicit check
Expand All @@ -295,6 +316,7 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b
# Add custom parameters
for k, v in custom_params.items():
command += [f"-{k}"]

# Skip only empty strings (pass flag with no value)
# Note: 0 and 0.0 are valid values, so use explicit check
if v != "" and v is not None:
Expand Down
70 changes: 70 additions & 0 deletions src/workflow/ParameterManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,52 @@
import shutil
import subprocess
import streamlit as st
import xml.etree.ElementTree as ET
from pathlib import Path


def bool_param_paths_from_param_xml_ini(ini_path: Path, tool_stem: str) -> set[str]:
"""
Return short parameter paths for every ``<ITEM type="bool">`` in a ParamXML .ini file.

Paths match the suffix after ``Tool:1:`` in pyOpenMS (e.g. ``algorithm:epd:masstrace_snr_filtering``).
"""
try:
root = ET.parse(ini_path).getroot()
except (ET.ParseError, OSError):
return set()

def local_tag(el: ET.Element) -> str:
t = el.tag
return t.rsplit("}", 1)[-1] if isinstance(t, str) and "}" in t else str(t)

out: set[str] = set()

def walk(el: ET.Element, parts: tuple[str, ...]) -> None:
for ch in el:
lt = local_tag(ch)
if lt == "NODE":
nm = ch.get("name") or ""
walk(ch, parts + (nm,))
elif lt == "ITEM" and (ch.get("type") or "").lower() == "bool":
nm = ch.get("name") or ""
segs = [p for p in parts if p]
if nm:
segs.append(nm)
if not segs:
continue
# Strip tool root NODE name and instance NODE "1" (not part of pyOpenMS short keys)
while segs and segs[0] in (tool_stem, "1"):
segs.pop(0)
if segs:
out.add(":".join(segs))

for ch in root:
if local_tag(ch) == "NODE":
walk(ch, ())
return out


class ParameterManager:
"""
Manages the parameters for a workflow, including saving parameters to a JSON file,
Expand All @@ -29,6 +73,29 @@ def __init__(self, workflow_dir: Path, workflow_name: str = None):
# Store workflow name for preset loading; default to directory stem if not provided
self.workflow_name = workflow_name or workflow_dir.stem

def bool_pairs_session_key(self) -> str:
"""Session state key holding a set of (tool name, param path) for bool TOPP params."""
return f"{self.ini_dir.parent.stem}-topp-bool-pairs"

def get_bool_param_pairs(self) -> set:
"""Return the cached set of (tool, param path) bool params; empty set if none."""
return st.session_state.get(self.bool_pairs_session_key(), set())

Comment on lines +80 to +83
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n src/workflow/ParameterManager.py | head -100

Repository: OpenMS/streamlit-template

Length of output: 4854


🏁 Script executed:

rg "get_bool_param_pairs" --type py -B 3 -A 3

Repository: OpenMS/streamlit-template

Length of output: 665


🏁 Script executed:

rg "get_bool_param_pairs\(\)" --type py

Repository: OpenMS/streamlit-template

Length of output: 51


🏁 Script executed:

find . -type f -name "*.py" | xargs grep -l "get_bool_param_pairs" 2>/dev/null

Repository: OpenMS/streamlit-template

Length of output: 104


🌐 Web query:

Streamlit st.session_state RuntimeError no active session

💡 Result:

Streamlit error “RuntimeError: no active session” (or similarly worded “Session state does not function when running a script without streamlit run”) happens when you access st.session_state outside an active Streamlit runtime session. Most common causes 1) Running the file as a normal Python script - st.session_state only works when the app is started by Streamlit (i.e., via streamlit run ...), because session state is tied to Streamlit’s WebSocket/runtime session. The Streamlit docs explain that session state is for “each user session” and is reset when the WebSocket/session is reset (e.g., refresh/navigation). [1] - If you run your script with python your_script.py, Streamlit emits a warning that session state does not function without streamlit run, followed by failures when you access attributes/keys. [2][3][4] 2) Code path executed during “startup”/external execution (e.g., debugger, background thread, import-time execution) - Community reports show that session state can be “reconstructed cleanly” or lost when code is executed outside the Streamlit-run context (e.g., debugging outside streamlit run). [5] 3) You’re in a session, but the specific key was never initialized yet - If you access a key that hasn’t been set in the current session, you’ll get a “no key … Did you forget to initialize it?” exception; this is separate from “no active session”, but it often appears during first run / first rerender. Initialize with a guard like if 'key' not in st.session_state: .... [1] How to fix A) Ensure you start the app correctly - Start with: - streamlit run path/to/app.py - Do not run with python app.py. This is exactly what the warning and error indicate. [2][4][3] B) Ensure session_state usage happens inside the Streamlit script execution - Avoid accessing/setting st.session_state at module import time if that code may run in a non-Streamlit context. - Avoid accessing session state from code that is executed outside Streamlit’s app lifecycle (e.g., separate background worker that is not managed by Streamlit’s runtime). Session state is tied to the Streamlit session/WebSocket. [1] C) Initialize keys before access - At the top of your app code (inside the Streamlit run), initialize keys before reading them, e.g.: - if 'foo' not in st.session_state: st.session_state['foo'] = default This is the documented initialization pattern. [1] Quick diagnosis checklist - Are you launching the app with streamlit run? If not, that’s the root cause. [2][3][4] - Do you see the warning about “Session state does not function when running a script without streamlit run”? That points directly to an inactive/no-runtime session. [2][6][3] - If you are definitely using streamlit run, check that the access is not happening in code that runs outside Streamlit’s lifecycle, and that you initialize keys with guards. [1][5]

Citations:


Guard get_bool_param_pairs() for non-Streamlit contexts.

The method calls st.session_state.get() which raises RuntimeError when no active Streamlit session exists. Since _merge_bool_params_from_ini() already guards against this case, apply the same pattern for consistency.

🔧 Suggested change
 def get_bool_param_pairs(self) -> set:
     """Return the cached set of (tool, param path) bool params; empty set if none."""
-    return st.session_state.get(self.bool_pairs_session_key(), set())
+    try:
+        return st.session_state.get(self.bool_pairs_session_key(), set())
+    except RuntimeError:
+        return set()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def get_bool_param_pairs(self) -> set:
"""Return the cached set of (tool, param path) bool params; empty set if none."""
return st.session_state.get(self.bool_pairs_session_key(), set())
def get_bool_param_pairs(self) -> set:
"""Return the cached set of (tool, param path) bool params; empty set if none."""
try:
return st.session_state.get(self.bool_pairs_session_key(), set())
except RuntimeError:
return set()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/workflow/ParameterManager.py` around lines 80 - 83, The
get_bool_param_pairs method currently calls st.session_state.get(...) which
throws RuntimeError outside of a Streamlit session; guard this call the same way
as _merge_bool_params_from_ini does (e.g., check for an active session or catch
RuntimeError) and return the default empty set when no session exists; update
get_bool_param_pairs to first verify Streamlit session availability before
accessing st.session_state.get using the same check/pattern used in
_merge_bool_params_from_ini so behavior is consistent.

def _merge_bool_params_from_ini(self, tool: str) -> None:
"""Load tool.ini (XML) and merge type=bool parameter paths into session_state."""
ini_path = Path(self.ini_dir, f"{tool}.ini")
if not ini_path.exists():
return
try:
sk = self.bool_pairs_session_key()
if sk not in st.session_state:
st.session_state[sk] = set()
for short in bool_param_paths_from_param_xml_ini(ini_path, tool):
st.session_state[sk].add((tool, short))
Comment on lines +90 to +94
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Replace per-tool bool metadata instead of only unioning.

Lines 93-94 only add entries. If a tool .ini changes during a session, removed bool params remain cached and may be emitted incorrectly as flags.

🔧 Suggested change
             sk = self.bool_pairs_session_key()
             if sk not in st.session_state:
                 st.session_state[sk] = set()
+            st.session_state[sk] = {
+                pair for pair in st.session_state[sk] if pair[0] != tool
+            }
             for short in bool_param_paths_from_param_xml_ini(ini_path, tool):
                 st.session_state[sk].add((tool, short))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/workflow/ParameterManager.py` around lines 90 - 94, The code currently
unions new bool params into the session set (using bool_pairs_session_key())
causing stale params to persist when a tool's .ini removes flags; instead,
compute the current set of bool pairs for the given tool by calling
bool_param_paths_from_param_xml_ini(ini_path, tool), remove any existing entries
in st.session_state[sk] that belong to this tool (matching the first element of
the tuple), then add the current tuples (tool, short) or simply replace
st.session_state[sk] entries for that tool with the new ones; update the logic
around bool_pairs_session_key(), bool_param_paths_from_param_xml_ini, ini_path
and tool to ensure removal of old tool-specific entries before adding the fresh
list.

except RuntimeError:
# No Streamlit session (e.g. plain `python` import)
pass

def create_ini(self, tool: str) -> bool:
"""
Create an ini file for a TOPP tool if it doesn't exist.
Expand All @@ -41,11 +108,14 @@ def create_ini(self, tool: str) -> bool:
"""
ini_path = Path(self.ini_dir, tool + ".ini")
if ini_path.exists():
self._merge_bool_params_from_ini(tool)
return True
try:
subprocess.call([tool, "-write_ini", str(ini_path)])
except FileNotFoundError:
return False
if ini_path.exists():
self._merge_bool_params_from_ini(tool)
return ini_path.exists()

def save_parameters(self) -> None:
Expand Down